-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix Zipkin spanFormat #5261
Fix Zipkin spanFormat #5261
Conversation
@@ -69,6 +69,9 @@ func startZipkinReceiver( | |||
} | |||
|
|||
consumerAdapter := newConsumerDelegate(logger, spanProcessor, tm) | |||
// reset Zipkin spanFormat | |||
consumerAdapter.batchConsumer.spanOptions.SpanFormat = processor.ZipkinSpanFormat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a test for this? TestZipkinReceiver
submits zipkin spans but uses a mockSpanProcessor
, we could potentially enhance it to validate that those spans are coming with the desired tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@yurishkuro PTAL :)
Signed-off-by: Yuan Fang <yuanfang@alauda.io>
ed0f593
to
77b5976
Compare
Looks like uploading codecov failed. |
<!-- !! Please DELETE this comment before posting. We appreciate your contribution to the Jaeger project! 👋🎉 --> ## Which problem is this PR solving? - <!-- Example: Resolves jaegertracing#123 --> Resolves jaegertracing#5260 ## Description of the changes - ## How was this change tested? - ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: Yuan Fang <yuanfang@alauda.io> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Vamshi Maskuri <gwcchintu@gmail.com>
This reverts commit 139b5f6.
<!-- !! Please DELETE this comment before posting. We appreciate your contribution to the Jaeger project! 👋🎉 --> ## Which problem is this PR solving? - <!-- Example: Resolves jaegertracing#123 --> Resolves jaegertracing#5260 ## Description of the changes - ## How was this change tested? - ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: Yuan Fang <yuanfang@alauda.io> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Vamshi Maskuri <gwcchintu@gmail.com>
This reverts commit 139b5f6. Signed-off-by: Vamshi Maskuri <gwcchintu@gmail.com>
Which problem is this PR solving?
Resolves #5260
Description of the changes
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test