From 1e1672e9b33a1e1eb450482a715f74ae71cc910e Mon Sep 17 00:00:00 2001 From: Suyog Soti Date: Thu, 25 Jul 2019 13:28:35 -0700 Subject: [PATCH] get rid of should only propagate --- .../pipeline/policies/distributed_tracing.py | 8 +----- sdk/core/azure-core/azure/core/settings.py | 4 --- .../azure-core/azure/core/tracing/common.py | 7 ----- .../azure/core/tracing/decorator.py | 2 +- .../azure/core/tracing/decorator_async.py | 2 +- .../test_tracing_decorator_async.py | 20 -------------- sdk/core/azure-core/tests/test_settings.py | 6 ++--- .../tests/test_tracing_decorator.py | 27 ------------------- .../azure-core/tests/test_tracing_policy.py | 6 +++-- sdk/core/azure-core/tests/tracing_common.py | 6 +---- 10 files changed, 10 insertions(+), 78 deletions(-) diff --git a/sdk/core/azure-core/azure/core/pipeline/policies/distributed_tracing.py b/sdk/core/azure-core/azure/core/pipeline/policies/distributed_tracing.py index 229b7d4380b5..ee6c1a36109f 100644 --- a/sdk/core/azure-core/azure/core/pipeline/policies/distributed_tracing.py +++ b/sdk/core/azure-core/azure/core/pipeline/policies/distributed_tracing.py @@ -77,11 +77,6 @@ def on_request(self, request): if parent_span is None: return - only_propagate = settings.tracing_should_only_propagate() - if only_propagate: - self.set_header(request, parent_span) - return - path = urlparse(request.http_request.url).path if not path: path = "/" @@ -96,8 +91,7 @@ def end_span(self, request, response=None): # type: (HttpRequest, Optional[HttpResponse]) -> None """Ends the span that is tracing the network and updates its status.""" span = tracing_context.current_span.get() # type: AbstractSpan - only_propagate = settings.tracing_should_only_propagate() - if span and not only_propagate: + if span is not None: span.set_http_attributes(request, response=response) request_id = request.headers.get(self._request_id) if request_id is not None: diff --git a/sdk/core/azure-core/azure/core/settings.py b/sdk/core/azure-core/azure/core/settings.py index e1b4c738cdb8..6bc71c9e850c 100644 --- a/sdk/core/azure-core/azure/core/settings.py +++ b/sdk/core/azure-core/azure/core/settings.py @@ -410,9 +410,5 @@ def _config(self, props): # pylint: disable=no-self-use "tracing_implementation", env_var="AZURE_SDK_TRACING_IMPLEMENTATION", convert=convert_tracing_impl, default=None ) - tracing_should_only_propagate = PrioritizedSetting( - "tracing_should_only_propagate", env_var="AZURE_TRACING_ONLY_PROPAGATE", convert=convert_bool, default=False - ) - settings = Settings() diff --git a/sdk/core/azure-core/azure/core/tracing/common.py b/sdk/core/azure-core/azure/core/tracing/common.py index f6e4716a2eb8..b0e425a14d10 100644 --- a/sdk/core/azure-core/azure/core/tracing/common.py +++ b/sdk/core/azure-core/azure/core/tracing/common.py @@ -83,10 +83,3 @@ def get_parent_span(parent_span): ) return parent_span - - -def should_use_trace(parent_span): - # type: (AbstractSpan) -> bool - """Given Parent Span Returns whether the function should be traced""" - only_propagate = settings.tracing_should_only_propagate() - return bool(parent_span and not only_propagate) diff --git a/sdk/core/azure-core/azure/core/tracing/decorator.py b/sdk/core/azure-core/azure/core/tracing/decorator.py index 9f647f3333c4..7d5d7bfce407 100644 --- a/sdk/core/azure-core/azure/core/tracing/decorator.py +++ b/sdk/core/azure-core/azure/core/tracing/decorator.py @@ -48,7 +48,7 @@ def wrapper_use_tracer(self, *args, **kwargs): original_span_instance = wrapper_class.get_current_span() parent_span = common.get_parent_span(passed_in_parent) ans = None - if common.should_use_trace(parent_span): + if parent_span is not None: common.set_span_contexts(parent_span) name = name_of_span or self.__class__.__name__ + "." + func.__name__ child = parent_span.span(name=name) diff --git a/sdk/core/azure-core/azure/core/tracing/decorator_async.py b/sdk/core/azure-core/azure/core/tracing/decorator_async.py index 1440f86525e2..781e20a6bef2 100644 --- a/sdk/core/azure-core/azure/core/tracing/decorator_async.py +++ b/sdk/core/azure-core/azure/core/tracing/decorator_async.py @@ -48,7 +48,7 @@ async def wrapper_use_tracer(self, *args, **kwargs): original_span_instance = wrapper_class.get_current_span() parent_span = common.get_parent_span(passed_in_parent) ans = None - if common.should_use_trace(parent_span): + if parent_span is not None: common.set_span_contexts(parent_span) name = name_of_span or self.__class__.__name__ + "." + func.__name__ child = parent_span.span(name=name) diff --git a/sdk/core/azure-core/tests/azure_core_asynctests/test_tracing_decorator_async.py b/sdk/core/azure-core/tests/azure_core_asynctests/test_tracing_decorator_async.py index 7fb903037d80..8b43ddf71a62 100644 --- a/sdk/core/azure-core/tests/azure_core_asynctests/test_tracing_decorator_async.py +++ b/sdk/core/azure-core/tests/azure_core_asynctests/test_tracing_decorator_async.py @@ -149,23 +149,3 @@ async def test_span_with_opencensus_complicated(value): assert parent.children[3].children[1].span_data.name == "MockClient.make_request" children = parent.children[1].children assert len(children) == 2 - - -@pytest.mark.asyncio -async def test_should_only_propagate(): - with ContextHelper(should_only_propagate=True): - exporter = MockExporter() - trace = tracer_module.Tracer(sampler=AlwaysOnSampler(), exporter=exporter) - with trace.start_span(name="OverAll") as parent: - client = MockClient() - await client.make_request(2) - with trace.span("child") as child: - await client.make_request(2, parent_span=parent) - assert OpenCensusSpan.get_current_span() == child - await client.make_request(2) - trace.finish() - exporter.build_tree() - parent = exporter.root - assert len(parent.children) == 1 - assert parent.children[0].span_data.name == "child" - assert not parent.children[0].children diff --git a/sdk/core/azure-core/tests/test_settings.py b/sdk/core/azure-core/tests/test_settings.py index 11e9dc48d523..1975c1adc6f3 100644 --- a/sdk/core/azure-core/tests/test_settings.py +++ b/sdk/core/azure-core/tests/test_settings.py @@ -217,20 +217,18 @@ def test_defaults(self): val = m.settings.defaults # assert isinstance(val, tuple) defaults = m.settings.config( - log_level=20, tracing_enabled=False, tracing_implementation=None, tracing_should_only_propagate=False + log_level=20, tracing_enabled=False, tracing_implementation=None ) assert val.log_level == defaults.log_level assert val.tracing_enabled == defaults.tracing_enabled assert val.tracing_implementation == defaults.tracing_implementation - assert val.tracing_should_only_propagate == defaults.tracing_should_only_propagate os.environ["AZURE_LOG_LEVEL"] = "debug" defaults = m.settings.config( - log_level=20, tracing_enabled=False, tracing_implementation=None, tracing_should_only_propagate=False + log_level=20, tracing_enabled=False, tracing_implementation=None ) assert val.log_level == defaults.log_level assert val.tracing_enabled == defaults.tracing_enabled assert val.tracing_implementation == defaults.tracing_implementation - assert val.tracing_should_only_propagate == defaults.tracing_should_only_propagate del os.environ["AZURE_LOG_LEVEL"] def test_current(self): diff --git a/sdk/core/azure-core/tests/test_tracing_decorator.py b/sdk/core/azure-core/tests/test_tracing_decorator.py index 873871f5b222..473060f6097a 100644 --- a/sdk/core/azure-core/tests/test_tracing_decorator.py +++ b/sdk/core/azure-core/tests/test_tracing_decorator.py @@ -105,15 +105,6 @@ def test_get_parent_span(self): should_be_old_parent = common.get_parent_span(parent.span_instance) assert should_be_old_parent.span_instance == parent.span_instance - def test_should_use_trace(self): - with ContextHelper(environ={"AZURE_TRACING_ONLY_PROPAGATE": "yes"}): - parent_span = OpenCensusSpan() - assert not common.should_use_trace(parent_span) - assert not common.should_use_trace(None) - parent_span = OpenCensusSpan() - assert common.should_use_trace(parent_span) - assert not common.should_use_trace(None) - class TestDecorator(object): def test_decorator_has_different_name(self): @@ -190,21 +181,3 @@ def test_span_with_opencensus_complicated(self, value): assert parent.children[3].children[1].span_data.name == "MockClient.make_request" children = parent.children[1].children assert len(children) == 2 - - def test_should_only_propagate(self): - with ContextHelper(should_only_propagate=True): - exporter = MockExporter() - trace = tracer_module.Tracer(sampler=AlwaysOnSampler(), exporter=exporter) - with trace.start_span(name="OverAll") as parent: - client = MockClient() - client.make_request(2) - with trace.span("child") as child: - client.make_request(2, parent_span=parent) - assert OpenCensusSpan.get_current_span() == child - client.make_request(2) - trace.finish() - exporter.build_tree() - parent = exporter.root - assert len(parent.children) == 1 - assert parent.children[0].span_data.name == "child" - assert not parent.children[0].children diff --git a/sdk/core/azure-core/tests/test_tracing_policy.py b/sdk/core/azure-core/tests/test_tracing_policy.py index 450baa2f4f1e..be7e1adf98e9 100644 --- a/sdk/core/azure-core/tests/test_tracing_policy.py +++ b/sdk/core/azure-core/tests/test_tracing_policy.py @@ -72,13 +72,15 @@ def test_distributed_tracing_policy_solo(should_set_sdk_context): assert network_span.span_data.attributes.get("http.status_code") == 504 -def test_distributed_tracing_policy_with_user_agent(): +@pytest.mark.parametrize("should_set_sdk_context", [True, False]) +def test_distributed_tracing_policy_with_user_agent(should_set_sdk_context): """Test policy working with user agent.""" with ContextHelper(environ={"AZURE_HTTP_USER_AGENT": "mytools"}): exporter = MockExporter() trace = tracer_module.Tracer(sampler=AlwaysOnSampler(), exporter=exporter) with trace.span("parent"): - tracing_context.current_span.set(OpenCensusSpan(trace.current_span())) + if should_set_sdk_context: + tracing_context.current_span.set(OpenCensusSpan(trace.current_span())) policy = DistributedTracingPolicy() request = HttpRequest("GET", "http://127.0.0.1") diff --git a/sdk/core/azure-core/tests/tracing_common.py b/sdk/core/azure-core/tests/tracing_common.py index 18f9b4e9f839..7fecf1d746f8 100644 --- a/sdk/core/azure-core/tests/tracing_common.py +++ b/sdk/core/azure-core/tests/tracing_common.py @@ -22,13 +22,12 @@ class ContextHelper(object): - def __init__(self, environ={}, tracer_to_use=None, should_only_propagate=None): + def __init__(self, environ={}, tracer_to_use=None): self.orig_tracer = OpenCensusSpan.get_current_tracer() self.orig_current_span = OpenCensusSpan.get_current_span() self.orig_sdk_context_span = tracing_context.current_span.get() self.os_env = mock.patch.dict(os.environ, environ) self.tracer_to_use = tracer_to_use - self.should_only_propagate = should_only_propagate def __enter__(self): self.orig_tracer = OpenCensusSpan.get_current_tracer() @@ -38,8 +37,6 @@ def __enter__(self): tracing_context.current_span.clear() if self.tracer_to_use is not None: settings.tracing_implementation.set_value(self.tracer_to_use) - if self.should_only_propagate is not None: - settings.tracing_should_only_propagate.set_value(self.should_only_propagate) self.os_env.start() execution_context.clear() tracing_context.current_span.clear() @@ -50,7 +47,6 @@ def __exit__(self, exc_type, exc_val, exc_tb): OpenCensusSpan.set_current_span(self.orig_current_span) tracing_context.current_span.set(self.orig_sdk_context_span) settings.tracing_implementation.unset_value() - settings.tracing_should_only_propagate.unset_value() self.os_env.stop()