Contribution of the resource providers to opentelemetry-java-contrib repo

[SylvainJuge]

Hi !

I just opened open-telemetry/opentelemetry-java-contrib#1074 in the java contrib repo after discussing the topic in the Java SIG meeting.

Ideally we'd like to have the most common cloud resource provider implementations available, would you be interested in contributing your implementation currently in detectors/resources to the open-telemetry/opentelemetry-java-contrib repo ?

If yes, would you also be interested in being maintainers of this component within otel ? That does not sound like a very strong commitment and you'll benefit from frequent opentelemetry release schedule and automatic version upgrades. I can definitely be a second co-maintainer of this component within otel if needed.

I saw that this project currently relies on the now obsolete io.opentelemetry:opentelemetry-sdk-extension-resources:1.19.0 dependency, which has now been moved to opentelemetry-java-contrib repository, as a consequence replacing this will already create a dependency to the contrib repo.
What I mean here is that ideally a single implementation in the contrib repo could easily be reused here without adding an extra repository as a dependency (just an extra artifact).

[SylvainJuge]

For now I see two potential aspects that could be problematic for re-use in OTel-based agents or SDKs:

• resource attributes fetched on demand and one by one, which means more than one HTTP request to the metadata endpoint which could make startup slower.
• currently targets Java 11, whereas upstream opentelemetry is java 8 compatible.

[dashpole]

Hey @SylvainJuge, thanks for reaching out. Overall, we would be happy to contribute the resource detector here to the java contrib repo.

One of our original reasons for having it in this repository was to be able to run presubmit integration tests on GCP infrastructure to ensure the resource detectors work as expected. For other languages, like golang, we ended up splitting the detector into a core detection library (https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/tree/main/detectors/gcp), and a contrib entrypoint (https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/gcp/detector.go). We can run integration tests on the core library to ensure the detection methods work properly, but the actual detector would live in the upstream contrib repository. Would something like that work for you?

From our side, @psx95 and @jsuereth would be code owners for the contrib detector, and @psx95 will work on the new detector in contrib, including working through the issues you mention.

[SylvainJuge]

I see, having integration test on your side definitely makes sense to ensure everything is working as expected in the target environment and to keep a short feedback loop.

If I understand correctly the plan you suggest involves:

• split the current resource detector in a "library" and a "resource detector"
• keep the library in this repository so you can easily maintain it and keep your integration tests
• contribute the resource detector to the opentelemetry contrib repo, this will just delegate to the library and means having a dependency on it.
• your current resource detector could be replaced by the contrib one (which delegates to the library).

From the perspective of a consumer of this GCP resource detector:

• the resource detector brings a dependency to the library
• the library should be minimal and should not bring extra dependencies* and be java 8 compatible.

* : for the AWS equivalent it used to use the complete AWS SDK and is now rewritten with a few HTTP queries.

Another alternative could be to keep everything in one "library+resource detector" in the contrib repo:

• also adds a dependency to the contrib repo
• you can keep your current integration tests as they are
• tests would still be required on the contrib repo, probably by mocking a few HTTP endpoints like existing resource detectors in the contrib repo.
• it will be released at the same frequency as other opentelemetry components

But that second option would likely introduce more friction than necessary on your side if you need to change anything the library, thus the first is probably better here.

Ideally, without going too deep into the implementation details, the "library" could be just about parsing the metadata, without any dependencies. That would help for testing without HTTP communication and using it with another HTTP client (see open-telemetry/opentelemetry-java-contrib#1061 for context).

[dashpole]

As a first step, @psx95 can work on a PR to extract the library into its own package here. Then, we can take a look at the dependencies, and see if it works, or if we need to explore other options.

The list of dependencies seems reasonable (at least no GCP SDKs, or anything), so hopefully it shouldn't be too hard:

opentelemetry-operations-java/detectors/resources/build.gradle

Lines 18 to 29 in 25e1b88

	dependencies {
	implementation(libraries.opentelemetry_api)
	implementation(libraries.opentelemetry_sdk)
	implementation(libraries.opentelemetry_sdk_autoconf)
	implementation(libraries.opentelemetry_semconv)
	implementation platform(libraries.opentelemetry_bom)
	testImplementation(testLibraries.assertj)
	testImplementation(testLibraries.junit)
	testImplementation(testLibraries.wiremock)
	testImplementation(testLibraries.mockito)
	testImplementation(testLibraries.opentelemetry_sdk_testing)
	}

[psx95]

I have a draft PR - #276 which refactors the platform detection logic from the resource providers and allows resource provider to rely on the library methods to retrieve the values for the attributes. (This is in-line with what our Go resource detector does)

However, which attributes to add to a particular resource after it has been detected would still be specified in the resource provider (proposed to go in contrib repo). Just wanted to call out the following effects of this approach -

1. The support library does not need to take dependency on OTel APIs, which means the upstream library can be kept up-to-date with latest APIs without being hindered by the need to update support library dependencies.
2. Our integration tests passing relies on correct detection logic (support lib) and correct attributes being present (upstream contrib repo).

Let me know if this approach works, and I can get toward finalizing the draft PR (notably, the unit tests are currently missing).

[psx95]

@SylvainJuge would you mind taking a look at #276 - it splits our resource detection into 2 modules -

1. support library: detectors/resources-support
2. upstream detector: detectors/resources

Let us know if this split works for the upstream contribution.

[SylvainJuge]

Hi @psx95, thanks for making progress on this!

Once this is merged and released, we can already reuse the otel resource detector as-is in any opentelemetry distribution so this is great.

The next logical step could be to contribute the detector directly into the opentelemetry-contrib-java repo, this contribution would then have a dependency on the support library here.

Do you have ideas how to implement it ?

Maybe instead of a direct contribution of the detector to the upstream contrib repo, I think it could be possible to have a direct dependency on the otel detector in this repo, hence allowing reuse without any code duplication. With that approach, everything would remain in this repository and that would be simpler. In a sense the upstream contrib module would just define a dependency to a released version of this repository and the maintenance burden would be minimal.

Also, from the perspective of the opentelemetry users, having a split between the otel detector and the resource library could be useful but not strictly requires as we would always have a direct dependency on the detector.

[psx95]

Hi @SylvainJuge, thank you for the review!

I was thinking of contributing the detector-resources module directly upstream. I was thinking this because -

1. Contributing the detector-resources upstream would enable the module to be benefitted by the regular updates to the OTel dependencies in the project. Any breaking change(s) or use of obsolete OTel libraries would be caught & fixed promptly.
2. It will be in-line with what we do with our go resource detector. (Not a major reason, but helps to keep things uniform IMO)

IIUC, the alternative you are suggesting is to add implementation in the upstream repo, which delegates all the logic/handling to the module in this dependency via a hard dependency (it would also be the only dependency). In this case, I feel that there could be potential discrepancies b/w the versions of OTel dependencies used in contrib, vs the module which would be maintained here.

cc: @dashpole @jsuereth

[SylvainJuge]

I agree with you @psx95 , the approach you are suggesting seems the most simple here.

However I wonder from your perspective how would the maintenance of the support library work, for example there is currently no direct consumer of this library without detector-resources, including tests.

What this means is that you will likely have to choose between:

• keep your tests as-is and use a dependency on the upstream contrib repo for testing the support library (not ideal, but acceptable short term solution)
• migrate all your tests to use the support library API directly, which is simpler and better, but requires more effort on your side.

[psx95]

Hi @SylvainJuge, I was thinking something along the lines of your first suggestion - using a dependency on the upstream contrib-repo, while replacing the transitive dependency on support library with the local version - to ensure testing on the local changes made in the support library.

This would help us catch any issues that arise due to any change within the GCP metadata servers and we can promptly update the support lib.

Since the upstream repo now only takes care of the ResourceAttributes mapping, I feel that having proper unit tests with mocks to ensure correct mapping should provide us with good enough coverage.

[psx95]

Created a PR in upstream open-telemetry java contrib repo adding the GCP Resource providers - open-telemetry/opentelemetry-java-contrib#1162

cc- @dashpole, @SylvainJuge

[psx95]

The linked PR is now merged upstream. Closing this issue as completed.