Skip to content

Conversation

jack-berg
Copy link
Member

Implement the declarative configuration ComponentProvider<Resource> SPI now that support has been added for resources in open-telemetry/opentelemetry-java#6625.

@jack-berg jack-berg requested a review from a team August 29, 2024 20:57
@github-actions github-actions bot requested a review from theletterf August 29, 2024 20:57
@github-actions github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label Aug 29, 2024
* href="https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/host.md#non-privileged-machine-id-lookup">the
* semantic conventions</a>
*/
public final class HostIdResourceProvider implements ConditionalResourceProvider {
Copy link
Member Author

Choose a reason for hiding this comment

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

This class didn't follow the pattern of the others. Instead of having a Resource HostIdResource.get() method for calling directly by users, and a separate HostIdResourceProvider for integration with autoconfigure, it only had autoconfigure support.

I've refactored it to follow the pattern used elsewhere, which makes it easy to add a ComponentProvider implementation in HostIdResourceComponentProvider.

Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed that there is no documentation for the JarServiceNameDetector or ManifestResourceProvider, and that these can't be used outside of autoconfigure.

@zeitlinger, since you wrote ManifestResourceProvider, were these omissions intentional?

Copy link
Member

Choose a reason for hiding this comment

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

no, I didn't think about this use case

* href="https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/host.md#non-privileged-machine-id-lookup">the
* semantic conventions</a>
*/
public final class HostIdResource {
Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing new in this class - just breaking out HostIdResourceProvider into two classes.

// this line is managed by .github/scripts/update-sdk-version.sh
val otelSdkVersion = "1.41.0"
// TODO: revert before merging
val otelSdkVersion = "1.42.0-SNAPSHOT"
Copy link
Member

Choose a reason for hiding this comment

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

reminder

Copy link
Member Author

Choose a reason for hiding this comment

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

Will resolve after #12186 is merged

@trask trask merged commit 2a7a553 into open-telemetry:main Sep 12, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test native This label can be applied to PRs to trigger them to run native tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants