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

Remove OtlpExporter resource modification code #5927

Closed
CodeBlanch opened this issue Oct 25, 2024 · 2 comments · Fixed by #6015
Closed

Remove OtlpExporter resource modification code #5927

CodeBlanch opened this issue Oct 25, 2024 · 2 comments · Fixed by #6015
Labels
bug Something isn't working good first issue Good for newcomers help wanted Good for taking. Extra help will be provided by maintainers pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Milestone

Comments

@CodeBlanch
Copy link
Member

We have some code in OtlpExporter which will add service name if it isn't found on the resource:

Discussed with @alanwest and neither of us feel this should be done. The OtlpExporter should just respect what the SDK resource has.

What needs to be done...

  • Remove this logic
  • Call it out as a breaking change on CHANGELOG
@CodeBlanch CodeBlanch added help wanted Good for taking. Extra help will be provided by maintainers good first issue Good for newcomers pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package labels Oct 25, 2024
@CodeBlanch CodeBlanch added this to the Future milestone Oct 25, 2024
@alanwest alanwest added the bug Something isn't working label Oct 25, 2024
@rajkumar-rangaraj
Copy link
Contributor

@CodeBlanch / @alanwest Could you please check if this logic also needs to be removed? There is similar code in the OTLP tag parser to populate peer.service. I don't see this as part of the semantic specification.

private struct TagEnumerationState : PeerServiceResolver.IPeerServiceState

Test:

@alanwest
Copy link
Member

alanwest commented Dec 5, 2024

There is similar code in the OTLP tag parser to populate peer.service.

Agreed that this should also be removed.

In conversation with @rajkumar-rangaraj he also raised the point that these changes are breaking. My opinion is that while these changes are breaking, I'd really just consider them bugs. Bug fixes usually change behavior, so much of the time they're "breaking", but I'd just call them out as bug fixes and not a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Good for taking. Extra help will be provided by maintainers pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Projects
None yet
3 participants