feat!(datadogexporter): added peer.service priority instead service.name #2817
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #2817 +/- ##
==========================================
- Coverage 91.60% 91.59% -0.01%
==========================================
Files 450 450
Lines 22191 22193 +2
==========================================
Hits 20327 20327
- Misses 1394 1395 +1
- Partials 470 471 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
angeliski
force-pushed
the
datadog-add-peer-service
branch
from
672b1e6 to
d0ecf61
Compare
peer.service is a user-defined name for a service, which is conceptually the same as a service name. We should use it when defined and not service.name
angeliski
force-pushed
the
datadog-add-peer-service
branch
from
d0ecf61 to
9f72d3c
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Hey @bogdandrutu @ericmustin could you help me here? Do I need to do something? |
|
@angeliski sorry for delay here, i didn't get any notifications on this. I think this change makes sense and fits the spec, lgtm |
ericmustin
approved these changes
Apr 1, 2021
|
@bogdandrutu could we merge this when there's a chance before next release is cut? |
tigrannajaryan
approved these changes
Apr 9, 2021
pmatyjasek-sumo
referenced
this pull request
in pmatyjasek-sumo/opentelemetry-collector-contrib
Apr 28, 2021
…ame (#2817) peer.service is a user-defined name for a service, which is conceptually the same as a service name. We should use it when defined and not service.name That behavior is because datadog need to "separate" the service: Before:  After:  You can see, when we don't prioritize the `peer.service`, the whole operation is showed in the **main** service (GET request and SETEX operation). After we change, the exporter could create the separation between operations and give the correct attribution The [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/a44d863edcdef63b0adce7b47df001933b7a158a/specification/trace/semantic_conventions/span-general.md#general-remote-service-attributes) says: > peer.service - The service.name of the remote service. SHOULD be equal to the actual service.name resource attribute of the remote service if any. So I think makes sense to change that behavior. I created a really simple implementation, so any feedback would be useful
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
peer.service is a user-defined name for a service, which is conceptually the same as a service name.
We should use it when defined and not service.name
That behavior is because datadog need to "separate" the service:
Before:
After:
You can see, when we don't prioritize the
peer.service, the whole operation is showed in the main service (GET request and SETEX operation). After we change, the exporter could create the separation between operations and give the correct attributionThe spec says:
So I think makes sense to change that behavior.
I created a really simple implementation, so any feedback would be useful
Link to tracking Issue:
Testing: Unit Test