Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove service name #1669

Merged
merged 33 commits into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
9ed9ef3
jaeger changes
dmarar Mar 1, 2021
6a0e03a
changes to remove service name from jaeger
dmarar Mar 2, 2021
830f071
merge changes from main
dmarar Mar 2, 2021
c96a6e8
more changes
dmarar Mar 2, 2021
979688c
changes for openconsensus
dmarar Mar 2, 2021
7882412
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
dmarar Mar 3, 2021
576e390
changes to remove servicename
dmarar Mar 3, 2021
2687e6e
changes in tests for reading value from env variable for resource
dmarar Mar 7, 2021
b11a838
merging from upstream/main
dmarar Mar 7, 2021
9c8344c
resolving conflicts
dmarar Mar 7, 2021
3daaf26
modified the jaeger tests
dmarar Mar 7, 2021
7859ad1
changes for removing the service_name
dmarar Mar 8, 2021
8b7ff0d
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
dmarar Mar 8, 2021
2b79f0a
lint fixes
dmarar Mar 8, 2021
a7ad8c4
re-phrase changelog.md
dmarar Mar 8, 2021
671c070
removing unwanted code
dmarar Mar 8, 2021
ce25dfe
changes to default service_name incase none
dmarar Mar 8, 2021
2aa4bff
fixing lint errors
dmarar Mar 8, 2021
cb5a00b
Merge branch 'main' into remove_service_name
dmarar Mar 8, 2021
eda668c
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
dmarar Mar 8, 2021
38e4f10
Merge branch 'remove_service_name' of https://github.com/dmarar/opent…
dmarar Mar 8, 2021
6b1423a
Merge branch 'main' into remove_service_name
dmarar Mar 9, 2021
c0869ec
Merge branch 'main' into remove_service_name
lzchen Mar 11, 2021
6b9641b
Merge branch 'main' into remove_service_name
lzchen Mar 12, 2021
f2cf6f5
code review changes
dmarar Mar 12, 2021
c70ce29
Merge branch 'remove_service_name' of https://github.com/dmarar/opent…
dmarar Mar 12, 2021
a29df32
removed commented OTEL_RESOURCE_ATTRIBUTES
dmarar Mar 12, 2021
6755b22
Merge branch 'main' into remove_service_name
dmarar Mar 15, 2021
5ead1b8
implementing review comments
dmarar Mar 15, 2021
8ab0294
fixing conflict in env variables because of another merge
dmarar Mar 16, 2021
d26c941
Merge branch 'main' into remove_service_name
lzchen Mar 16, 2021
be2cdb2
Merge branch 'main' into remove_service_name
Mar 16, 2021
dda78a9
Merge branch 'main' into remove_service_name
lzchen Mar 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#1500](https://github.com/open-telemetry/opentelemetry-python/pull/1500))

### Changed
- remove `service_name` from constructor of jaeger and opencensus exporters and
use of env variable `OTEL_PYTHON_SERVICE_NAME`
([#1669])(https://github.com/open-telemetry/opentelemetry-python/pull/1669)
- Rename `IdsGenerator` to `IdGenerator`
([#1651])(https://github.com/open-telemetry/opentelemetry-python/pull/1651)
- Make TracerProvider's resource attribute private
Expand Down
4 changes: 1 addition & 3 deletions docs/examples/opencensus-exporter-tracer/collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor

exporter = OpenCensusSpanExporter(
service_name="basic-service", endpoint="localhost:55678"
)
exporter = OpenCensusSpanExporter(endpoint="localhost:55678")

trace.set_tracer_provider(TracerProvider())
tracer = trace.get_tracer(__name__)
Expand Down
11 changes: 7 additions & 4 deletions docs/getting_started/jaeger_example.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,18 @@
# jaeger_example.py
from opentelemetry import trace
from opentelemetry.exporter import jaeger
from opentelemetry.sdk.resources import SERVICE_NAME, Resource
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor

trace.set_tracer_provider(TracerProvider())
trace.set_tracer_provider(
TracerProvider(
resource=Resource.create({SERVICE_NAME: "my-helloworld-service"})
)
)

jaeger_exporter = jaeger.JaegerExporter(
service_name="my-helloworld-service",
agent_host_name="localhost",
agent_port=6831,
agent_host_name="localhost", agent_port=6831,
)

trace.get_tracer_provider().add_span_processor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

# create a JaegerExporter
jaeger_exporter = jaeger.JaegerExporter(
service_name="my-helloworld-service",
# configure agent
agent_host_name="localhost",
agent_port=6831,
Expand All @@ -29,7 +28,6 @@
# `EXPORTER_JAEGER_CERTIFICATE` with file containing creds.

# jaeger_exporter = jaeger.JaegerExporter(
# service_name="my-helloworld-service",
# collector_endpoint="localhost:14250",
# insecure=True,
# transport_format="protobuf",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

# create a JaegerExporter
jaeger_exporter = jaeger.JaegerExporter(
service_name='my-helloworld-service',
# configure agent
agent_host_name='localhost',
agent_port=6831,
Expand Down Expand Up @@ -72,6 +71,7 @@

from grpc import ChannelCredentials, insecure_channel, secure_channel

from opentelemetry import trace
from opentelemetry.exporter.jaeger import util
from opentelemetry.exporter.jaeger.gen import model_pb2
from opentelemetry.exporter.jaeger.gen.collector_pb2 import PostSpansRequest
Expand All @@ -91,6 +91,7 @@
OTEL_EXPORTER_JAEGER_PASSWORD,
OTEL_EXPORTER_JAEGER_USER,
)
from opentelemetry.sdk.resources import SERVICE_NAME, Resource
from opentelemetry.sdk.trace.export import SpanExporter, SpanExportResult

DEFAULT_AGENT_HOST_NAME = "localhost"
Expand All @@ -109,8 +110,6 @@ class JaegerExporter(SpanExporter):
"""Jaeger span exporter for OpenTelemetry.

Args:
service_name: Service that logged an annotation in a trace.Classifier
when query for spans.
agent_host_name: The host name of the Jaeger-Agent.
agent_port: The port of the Jaeger-Agent.
collector_endpoint: The endpoint of the Jaeger collector that uses
Expand All @@ -128,7 +127,6 @@ class JaegerExporter(SpanExporter):

def __init__(
self,
service_name: str,
agent_host_name: Optional[str] = None,
agent_port: Optional[int] = None,
collector_endpoint: Optional[str] = None,
Expand All @@ -140,7 +138,6 @@ def __init__(
max_tag_value_length: Optional[int] = None,
udp_split_oversized_batches: bool = None,
):
self.service_name = service_name
self._max_tag_value_length = max_tag_value_length
self.agent_host_name = _parameter_setter(
param=agent_host_name,
Expand Down Expand Up @@ -194,6 +191,12 @@ def __init__(
if transport_format
else TRANSPORT_FORMAT_THRIFT
)
tracer_provider = trace.get_tracer_provider()
self.service_name = (
tracer_provider.resource.attributes[SERVICE_NAME]
if getattr(tracer_provider, "resource", None)
else Resource.create({})
dmarar marked this conversation as resolved.
Show resolved Hide resolved
)

@property
def _collector_grpc_client(self) -> Optional[CollectorServiceStub]:
Expand Down Expand Up @@ -234,6 +237,7 @@ def _collector_http_client(self) -> Optional[Collector]:
return self._collector

def export(self, spans) -> SpanExportResult:

translator = Translate(spans)
if self.transport_format == TRANSPORT_FORMAT_PROTOBUF:
pb_translator = ProtobufTranslator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,20 @@
from opentelemetry.sdk.environment_variables import (
OTEL_EXPORTER_JAEGER_CERTIFICATE,
OTEL_EXPORTER_JAEGER_ENDPOINT,
OTEL_RESOURCE_ATTRIBUTES,
)
from opentelemetry.sdk.trace import Resource
from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
from opentelemetry.trace.status import Status, StatusCode


class Provider:
def __init__(self, resource=None, id_generator=None):
self.id_generator = id_generator
self.processor = None
self.resource = Resource.create({})


# pylint:disable=no-member
class TestJaegerExporter(unittest.TestCase):
def setUp(self):
Expand All @@ -51,6 +59,7 @@ def setUp(self):
self._test_span = trace._Span("test_span", context=context)
self._test_span.start()
self._test_span.end()

# pylint: disable=protected-access

def test_constructor_by_environment_variables(self):
Expand All @@ -66,21 +75,19 @@ def test_constructor_by_environment_variables(self):
OTEL_EXPORTER_JAEGER_ENDPOINT: collector_endpoint,
OTEL_EXPORTER_JAEGER_CERTIFICATE: os.path.dirname(__file__)
+ "/certs/cred.cert",
OTEL_RESOURCE_ATTRIBUTES: "service.name=my-opentelemetry-jaeger",
},
)

env_patch.start()

exporter = JaegerExporter(
service_name=service, transport_format="protobuf"
)

self.assertEqual(exporter.service_name, service)
dmarar marked this conversation as resolved.
Show resolved Hide resolved
self.assertIsNotNone(exporter._collector_grpc_client)
self.assertEqual(exporter.collector_endpoint, collector_endpoint)
self.assertIsNotNone(exporter.credentials)

env_patch.stop()
with patch("opentelemetry.exporter.jaeger.trace") as mock_trace:
env_patch.start()
mock_trace.get_tracer_provider.return_value = Provider()
# trace_api.set_tracer_provider(TracerProvider())
exporter = JaegerExporter(transport_format="protobuf")
self.assertEqual(exporter.service_name, service)
self.assertIsNotNone(exporter._collector_grpc_client)
self.assertEqual(exporter.collector_endpoint, collector_endpoint)
self.assertIsNotNone(exporter.credentials)
env_patch.stop()

# pylint: disable=too-many-locals,too-many-statements
def test_translate_to_jaeger(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import unittest
from unittest import mock
from unittest.mock import patch

# pylint:disable=no-name-in-module
# pylint:disable=import-error
Expand All @@ -30,13 +31,22 @@
OTEL_EXPORTER_JAEGER_ENDPOINT,
OTEL_EXPORTER_JAEGER_PASSWORD,
OTEL_EXPORTER_JAEGER_USER,
OTEL_RESOURCE_ATTRIBUTES,
)
from opentelemetry.sdk.trace import Resource
from opentelemetry.sdk.resources import SERVICE_NAME
from opentelemetry.sdk.trace import Resource, TracerProvider
from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
from opentelemetry.trace import SpanKind
from opentelemetry.trace.status import Status, StatusCode


class Provider:
def __init__(self, resource=None, id_generator=None):
self.id_generator = id_generator
self.processor = None
self.resource = Resource.create({})


dmarar marked this conversation as resolved.
Show resolved Hide resolved
class TestJaegerExporter(unittest.TestCase):
def setUp(self):
# create and save span to be used in tests
Expand All @@ -57,17 +67,24 @@ def test_constructor_default(self):
service_name = "my-service-name"
agent_host_name = "localhost"
agent_port = 6831
exporter = jaeger_exporter.JaegerExporter(service_name)

self.assertEqual(exporter.service_name, service_name)
self.assertEqual(exporter.agent_host_name, agent_host_name)
self.assertEqual(exporter.agent_port, agent_port)
self.assertEqual(exporter.collector_endpoint, None)
self.assertEqual(exporter.username, None)
self.assertEqual(exporter.password, None)
self.assertTrue(exporter._collector_http_client is None)
self.assertTrue(exporter._agent_client is not None)
self.assertIsNone(exporter._max_tag_value_length)
env_patch = patch.dict(
"os.environ",
{OTEL_RESOURCE_ATTRIBUTES: "service.name=my-service-name"},
)
with patch("opentelemetry.exporter.jaeger.trace") as mock_trace:
env_patch.start()
mock_trace.get_tracer_provider.return_value = Provider()
exporter = jaeger_exporter.JaegerExporter()
self.assertEqual(exporter.service_name, service_name)
self.assertEqual(exporter.agent_host_name, agent_host_name)
self.assertEqual(exporter.agent_port, agent_port)
self.assertEqual(exporter.collector_endpoint, None)
self.assertEqual(exporter.username, None)
self.assertEqual(exporter.password, None)
self.assertTrue(exporter._collector_http_client is None)
self.assertTrue(exporter._agent_client is not None)
self.assertIsNone(exporter._max_tag_value_length)
env_patch.stop()

def test_constructor_explicit(self):
# pylint: disable=protected-access
Expand All @@ -81,32 +98,37 @@ def test_constructor_explicit(self):
username = "username"
password = "password"
auth = (username, password)

exporter = jaeger_exporter.JaegerExporter(
service_name=service,
agent_host_name=agent_host_name,
agent_port=agent_port,
collector_endpoint=collector_endpoint,
username=username,
password=password,
max_tag_value_length=42,
env_patch = patch.dict(
dmarar marked this conversation as resolved.
Show resolved Hide resolved
"os.environ",
{OTEL_RESOURCE_ATTRIBUTES: "service.name=my-opentelemetry-jaeger"},
)

self.assertEqual(exporter.service_name, service)
self.assertEqual(exporter.agent_host_name, agent_host_name)
self.assertEqual(exporter.agent_port, agent_port)
self.assertTrue(exporter._collector_http_client is not None)
self.assertEqual(exporter._collector_http_client.auth, auth)
# property should not construct new object
collector = exporter._collector_http_client
self.assertEqual(exporter._collector_http_client, collector)
# property should construct new object
exporter._collector = None
exporter.username = None
exporter.password = None
self.assertNotEqual(exporter._collector_http_client, collector)
self.assertTrue(exporter._collector_http_client.auth is None)
self.assertEqual(exporter._max_tag_value_length, 42)
with patch("opentelemetry.exporter.jaeger.trace") as mock_trace:
env_patch.start()
mock_trace.get_tracer_provider.return_value = Provider()
exporter = jaeger_exporter.JaegerExporter(
agent_host_name=agent_host_name,
agent_port=agent_port,
collector_endpoint=collector_endpoint,
username=username,
password=password,
max_tag_value_length=42,
)
self.assertEqual(exporter.service_name, service)
self.assertEqual(exporter.agent_host_name, agent_host_name)
self.assertEqual(exporter.agent_port, agent_port)
self.assertTrue(exporter._collector_http_client is not None)
self.assertEqual(exporter._collector_http_client.auth, auth)
# property should not construct new object
collector = exporter._collector_http_client
self.assertEqual(exporter._collector_http_client, collector)
# property should construct new object
exporter._collector = None
exporter.username = None
exporter.password = None
self.assertNotEqual(exporter._collector_http_client, collector)
self.assertTrue(exporter._collector_http_client.auth is None)
self.assertEqual(exporter._max_tag_value_length, 42)
env_patch.stop()

def test_constructor_by_environment_variables(self):
# pylint: disable=protected-access
Expand All @@ -130,30 +152,32 @@ def test_constructor_by_environment_variables(self):
OTEL_EXPORTER_JAEGER_ENDPOINT: collector_endpoint,
OTEL_EXPORTER_JAEGER_USER: username,
OTEL_EXPORTER_JAEGER_PASSWORD: password,
OTEL_RESOURCE_ATTRIBUTES: "service.name=my-opentelemetry-jaeger",
},
)

environ_patcher.start()

exporter = jaeger_exporter.JaegerExporter(service_name=service)

self.assertEqual(exporter.service_name, service)
self.assertEqual(exporter.agent_host_name, agent_host_name)
self.assertEqual(exporter.agent_port, int(agent_port))
self.assertTrue(exporter._collector_http_client is not None)
self.assertEqual(exporter.collector_endpoint, collector_endpoint)
self.assertEqual(exporter._collector_http_client.auth, auth)
# property should not construct new object
collector = exporter._collector_http_client
self.assertEqual(exporter._collector_http_client, collector)
# property should construct new object
exporter._collector = None
exporter.username = None
exporter.password = None
self.assertNotEqual(exporter._collector_http_client, collector)
self.assertTrue(exporter._collector_http_client.auth is None)

environ_patcher.stop()
with patch("opentelemetry.exporter.jaeger.trace") as mock_trace:
environ_patcher.start()
mock_trace.get_tracer_provider.return_value = Provider()
# trace_api.set_tracer_provider(TracerProvider())
exporter = jaeger_exporter.JaegerExporter()
self.assertEqual(exporter.service_name, service)
self.assertEqual(exporter.agent_host_name, agent_host_name)
self.assertEqual(exporter.agent_port, int(agent_port))
self.assertTrue(exporter._collector_http_client is not None)
self.assertEqual(exporter.collector_endpoint, collector_endpoint)
self.assertEqual(exporter._collector_http_client.auth, auth)
# property should not construct new object
collector = exporter._collector_http_client
self.assertEqual(exporter._collector_http_client, collector)
# property should construct new object
exporter._collector = None
exporter.username = None
exporter.password = None
self.assertNotEqual(exporter._collector_http_client, collector)
self.assertTrue(exporter._collector_http_client.auth is None)

environ_patcher.stop()

def test_nsec_to_usec_round(self):
# pylint: disable=protected-access
Expand Down Expand Up @@ -432,9 +456,17 @@ def test_translate_to_jaeger(self):
self.assertEqual(spans, expected_spans)

def test_export(self):

"""Test that agent and/or collector are invoked"""

trace_api.set_tracer_provider(
TracerProvider(
resource=Resource.create({SERVICE_NAME: "text_export"})
)
)

exporter = jaeger_exporter.JaegerExporter(
"test_export", agent_host_name="localhost", agent_port=6318
agent_host_name="localhost", agent_port=6318
)

# just agent is configured now
Expand Down
Loading