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

datadog-exporter: bugfix for unintentional type change #261

Merged
merged 4 commits into from
Jan 5, 2021

Conversation

alertedsnake
Copy link
Contributor

@alertedsnake alertedsnake commented Dec 17, 2020

Description

Using |= here makes trace_flags into an int, rather than a TraceFlags object:

>>> import opentelemetry.trace as trace
>>> f = trace.TraceFlags()
>>> type(f)
<class 'opentelemetry.trace.span.TraceFlags'>
>>> f |= trace.TraceFlags.SAMPLED
>>> type(f)
<class 'int'>

This fix will preserve the type. I assume this is desired - can't see why it wouldn't be, but I'm still a bit new to this stack, so confirmation would be welcome.

You may ask, but where's the sample code to demonstrate the bug? I'm in the middle of trying to debug a number of odd issues with the gRPC client interceptor when used with Datadog, so it's a bit convoluted - essentially I'm making a gRPC request to one service which makes a gRPC request to another. When the Datadog exporter is enabled, I get a traceback like this:

Traceback (most recent call last):
  File "/home/michael/work/myapp/.venv/lib/python3.8/site-packages/grpc/_server.py", line 435, in _call_behavior
    response_or_iterator = behavior(argument, context)
  File "/home/michael/xfer/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py", line 252, in telemetry_interceptor
    with self._start_span(
  File "/home/michael/xfer/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py", line 232, in _start_span
    return self._tracer.start_as_current_span(
  File "/home/michael/xfer/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py", line 756, in start_as_current_span
    span = self.start_span(
  File "/home/michael/xfer/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py", line 807, in start_span
    sampling_result = self.sampler.should_sample(
  File "/home/michael/xfer/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py", line 273, in should_sample
    if parent_span_context.trace_flags.sampled:
AttributeError: 'int' object has no attribute 'sampled'

If I switch to the Jaeger exporter rather than Datadog, my code runs perfectly fine.

If I remove this code from my primary service, it also runs fine:

from opentelemetry.exporter.datadog.propagator import DatadogFormat
from opentelemetry import propagators
propagators.set_global_textmap(DatadogFormat())

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

The best way is to just look at what the code does in the console.

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added

@alertedsnake alertedsnake requested a review from a team as a code owner December 17, 2020 05:53
@alertedsnake alertedsnake requested review from owais and ocelotl and removed request for a team December 17, 2020 05:53
Michael Stella and others added 2 commits December 18, 2020 14:37
Using `|=` here makes `trace_flags` into an int, rather than a
`TraceFlags` object:

```
>>> import opentelemetry.trace as trace
>>> f = trace.TraceFlags()
>>> type(f)
<class 'opentelemetry.trace.span.TraceFlags'>
>>> f |= trace.TraceFlags.SAMPLED
>>> type(f)
<class 'int'>
```

This fix will preserve the type.

You may ask, but where's the sample code to demostrate the bug?  I'm in
the middle of trying to debug a number of odd issues with the gRPC
client interceptor, so it's a bit convoluted, but it looks like this:

```
Traceback (most recent call last):
  File "/home/michael/work/myapp/.venv/lib/python3.8/site-packages/grpc/_server.py", line 435, in _call_behavior
    response_or_iterator = behavior(argument, context)
  File "/home/michael/xfer/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py", line 252, in telemetry_interceptor
    with self._start_span(
  File "/home/michael/xfer/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py", line 232, in _start_span
    return self._tracer.start_as_current_span(
  File "/home/michael/xfer/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py", line 756, in start_as_current_span
    span = self.start_span(
  File "/home/michael/xfer/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py", line 807, in start_span
    sampling_result = self.sampler.should_sample(
  File "/home/michael/xfer/opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py", line 273, in should_sample
    if parent_span_context.trace_flags.sampled:
AttributeError: 'int' object has no attribute 'sampled'
```
@codeboten codeboten merged commit 5a6c4f6 into open-telemetry:master Jan 5, 2021
@alertedsnake alertedsnake deleted the fix-datadog-bug branch January 5, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants