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

Make IResourceDetector public to allow custom implementations of resource detectors. #2897

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

Yun-Ting
Copy link
Contributor

@Yun-Ting Yun-Ting commented Feb 14, 2022

Related to #2752.

Changes

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@Yun-Ting Yun-Ting requested a review from a team February 14, 2022 23:54
@codecov
Copy link

codecov bot commented Feb 15, 2022

Codecov Report

Merging #2897 (471f56f) into main (9232e56) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2897      +/-   ##
==========================================
+ Coverage   84.21%   84.22%   +0.01%     
==========================================
  Files         251      251              
  Lines        8883     8883              
==========================================
+ Hits         7481     7482       +1     
+ Misses       1402     1401       -1     
Impacted Files Coverage Δ
...ZPages/Implementation/ZPagesExporterEventSource.cs 62.50% <0.00%> (+6.25%) ⬆️

@alanwest
Copy link
Member

I updated the description so that merging would not auto close the linked issue. This PR doesn't resolve the issue but rather enables implementing the issue.

@alanwest alanwest merged commit 84821ff into open-telemetry:main Feb 15, 2022
@cijothomas
Copy link
Member

A good immediate follow up would be to document this for https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/trace/extending-the-sdk . (The doc is tied to tracing, we need to separate it out to more common place, but that can be a separate issue.) @Yun-Ting Could you create an issue for these and/or propose a doc update with this?

@cijothomas
Copy link
Member

I updated the description so that merging would not auto close the linked issue. This PR doesn't resolve the issue but rather enables implementing the issue.

Shipping actual detectors could be something for the -contrib repo? We can move the issue to -contrib repo.

@Yun-Ting
Copy link
Contributor Author

I updated the description so that merging would not auto close the linked issue. This PR doesn't resolve the issue but rather enables implementing the issue.

Thanks Alan 🤗! I'll be more careful the next time.

@Yun-Ting Yun-Ting changed the title Make IResourceDetector public to allow custom implementations of resource detectors. Publicize required methods to allow for custom implementations of resource detectors. Feb 28, 2022
@Yun-Ting Yun-Ting changed the title Publicize required methods to allow for custom implementations of resource detectors. Make IResourceDetector public to allow custom implementations of resource detectors. Feb 28, 2022
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