-
Notifications
You must be signed in to change notification settings - Fork 276
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
ResourceDetectors - Host #1507
ResourceDetectors - Host #1507
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1507 +/- ##
==========================================
- Coverage 73.91% 73.36% -0.56%
==========================================
Files 267 255 -12
Lines 9615 9422 -193
==========================================
- Hits 7107 6912 -195
- Misses 2508 2510 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
build-test-host: | ||
needs: detect-changes | ||
if: | | ||
contains(needs.detect-changes.outputs.changes, 'host') |
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.
"host" is too generic? Maybe something more narrow like ResourceDetector.host, if feasible.
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.
It is in sync with current pattern. Will be handled in scope of #1513 for all packages (for process we have already names conflict).
<MinVerTagPrefix>ResourceDetectors.Host-</MinVerTagPrefix> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<PackageReference Include="OpenTelemetry" Version="1.6.0" /> |
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 use OTelSdkVersion
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.
From PR description:
The plan is to release shortly as alpha with reference to OTel 1.6.0.
Then create follow up help-wanted issue to extend it for another attributes and update OTel to 1.7.0.
Will do, just after initial release.
Fixes N/A
Changes
Initial implementation for
host.name
detector from https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/host.md (this attribute is what I need for now).The plan is to release shortly as alpha with reference to OTel 1.6.0.
Then create follow up
help-wanted
issue to extend it for another attributes and update OTel to 1.7.0.For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes[ ] Design discussion issue #Notes
Similar pr for process: #1506