Skip to content

Fixed debug field usage in zipkin exporter#549

Merged
c24t merged 1 commit intoopen-telemetry:masterfrom
owais:patch-1
Apr 4, 2020
Merged

Fixed debug field usage in zipkin exporter#549
c24t merged 1 commit intoopen-telemetry:masterfrom
owais:patch-1

Conversation

@owais
Copy link
Copy Markdown
Contributor

@owais owais commented Apr 3, 2020

Zipkin spec expects the debug field to be a boolean, not an integer. OpenTelemetry collector rejects spans exported by the Python Zipkin exporter with the following error message:

Traces cannot be uploaded; status code: 400, message json: cannot unmarshal number into Go struct field .debug of type bool

Zipkin data model confirms that the field should be a boolean here: https://zipkin.io/zipkin-api/

@owais owais requested a review from a team April 3, 2020 21:24
Zipkin spec expects the debug field to be a boolean, not an integer. OpenTelemetry collector rejects spans exported by the Python Zipkin exporter with the following error message:

```
Traces cannot be uploaded; status code: 400, message json: cannot unmarshal number into Go struct field .debug of type bool
```

Zipkin data model confirm that the field should be a boolean here: https://zipkin.io/zipkin-api/
Copy link
Copy Markdown
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. LGTM!

Copy link
Copy Markdown
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@c24t c24t merged commit 33a75d0 into open-telemetry:master Apr 4, 2020
@codefromthecrypt
Copy link
Copy Markdown
Contributor

sampled != debug though

https://github.com/openzipkin/b3-propagation

            if context.trace_flags.sampled:
                zipkin_span["debug"] = True

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.

4 participants