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

[Api] support for adding default tags to tracer #6137

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

alimahboubi
Copy link

Fixes #6016
Design discussion issue #6016

Changes

This pull request adds a new overload to the TracerProvider.GetTracer method to support the inclusion of tags, similar to the new ActivitySource constructor overload in .NET 9. This allows users to associate initial tags with a Tracer (and therefore the underlying ActivitySource), which can be used to differentiate application instances with different configurations. These tags are applied to all activities created by the ActivitySource.

The new overload is:

public Tracer GetTracer(string name, string? version = null, IEnumerable<KeyValuePair<string, object?>>? tags = null)

@alimahboubi alimahboubi requested a review from a team as a code owner February 15, 2025 20:00
@github-actions github-actions bot added the pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package label Feb 15, 2025
@Kielek
Copy link
Contributor

Kielek commented Feb 18, 2025

@alimahboubi, it is good practice to execute build locally. In this case it should be done in Release mode. For now, I am not able to compile the code.

Could you please fix it?

- update "PublicAPI.Shipped" file
@alimahboubi
Copy link
Author

@Kielek Thanks for pointing that out! I've fixed the compilation error. I've also run a local build in Release mode to confirm

- Moved new GetTracer overload to the Unshipped file.
- Updated GetTracer signature to include nullable `version` and `tags` parameters.
- Removed the default parameter value from the previous line (existing GetTracer overload) as it was redundant after adding the new overload.
@alimahboubi
Copy link
Author

alimahboubi commented Feb 20, 2025

@Kielek Thanks for the detailed feedback! I've incorporated both of your suggestions:

  • The new GetTracer overload is now in the Unshipped file.

  • I've updated the GetTracer signature to include the optional version and tags parameters, and adjusted the existing overload accordingly (removing the default parameter).

  • I've updated the public contract part.

I've updated the commit with these changes. Please take another look!

@alimahboubi alimahboubi requested a review from Kielek February 20, 2025 13:14
Copy link
Contributor

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

Your assertions are to weak. The goal is to have one instance per tags set. Now, you have multiple instances for the same set of tags.

If you include all changes, tests will be failing. You need to fix the TracerKey.

@alimahboubi alimahboubi requested a review from Kielek February 21, 2025 11:19
Copy link
Contributor

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

Worth to add entry in CHANGELOG under OpenTelemetry.Api

@Kielek Kielek changed the title Feature/support for adding default tags to tracer [Api] support for adding default tags to tracer Feb 21, 2025
@alimahboubi alimahboubi requested a review from Kielek February 21, 2025 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] Support for adding default tags to specific Tracer
2 participants