Skip to content

add missing declarative config resource providers #14222

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 29 commits into from
Sep 11, 2025
Merged

add missing declarative config resource providers #14222

merged 29 commits into from
Sep 11, 2025

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Jul 10, 2025 •
edited
Loading

Fixes #14081

@zeitlinger zeitlinger self-assigned this Jul 10, 2025
@zeitlinger zeitlinger requested a review from a team as a code owner July 10, 2025 14:37
@zeitlinger zeitlinger mentioned this pull request Jul 10, 2025
Closed
@zeitlinger zeitlinger marked this pull request as draft July 11, 2025 07:07
@zeitlinger zeitlinger force-pushed the declarative-config-resource-providers branch from f5600d0 to 6728f62 Compare July 15, 2025 15:22
@zeitlinger zeitlinger marked this pull request as ready for review July 15, 2025 16:09
@zeitlinger zeitlinger moved this from In Progress to Awaiting Review in Declarative Configuration: Java Jul 15, 2025
@zeitlinger
Copy link
Member Author

@laurit please have a look 😄

laurit
laurit reviewed Jul 16, 2025
View reviewed changes
@zeitlinger zeitlinger mentioned this pull request Jul 17, 2025
Open
@zeitlinger
Copy link
Member Author

@laurit can you check again?

@zeitlinger zeitlinger force-pushed the declarative-config-resource-providers branch from f283f17 to e99c20a Compare July 22, 2025 08:08
@zeitlinger
Copy link
Member Author

@laurit can you check again?

1 similar comment
@zeitlinger
Copy link
Member Author

@laurit can you check again?

@zeitlinger zeitlinger force-pushed the declarative-config-resource-providers branch 2 times, most recently from 78f8138 to 2a610ea Compare August 18, 2025 07:39
@zeitlinger
Copy link
Member Author

@trask can you check again?

trask
trask reviewed Aug 20, 2025
View reviewed changes
@zeitlinger
Copy link
Member Author

@trask I've separated the "extraction" logic (which is re-usable) from the declarative and non-declarative provider interfaces, which should make the code easier to maintain (e.g. to delete the non-declarative classes)

@otelbot-java-instrumentation
Copy link
Contributor

🔧 The result from spotlessApply was committed to the PR branch.

@zeitlinger zeitlinger force-pushed the declarative-config-resource-providers branch from 66b0827 to 27bcee2 Compare August 29, 2025 13:39
@otelbot-java-instrumentation
Copy link
Contributor

🔧 The result from spotlessApply was committed to the PR branch.

@zeitlinger
Copy link
Member Author

@trask can you check again?

@zeitlinger
Copy link
Member Author

where can I add an integration test tests all resource providers, including a customizer in javaagent-tooling.

I put it in https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/14222/files#diff-a437e5890ee916a520c44636f60217781d18909c5b5515b54f882df75ce7100a for now.

@laurit do you have an idea?

@zeitlinger zeitlinger force-pushed the declarative-config-resource-providers branch from 6acc35c to 194d2cb Compare September 8, 2025 10:02
@zeitlinger zeitlinger added this to the v2.20.0 milestone Sep 11, 2025
@zeitlinger
Copy link
Member Author

@laurit @trask can we get this into the next release?

This would allow us to have milestone 1 ready for the release after that.

@zeitlinger
Copy link
Member Author

@laurit please check again

trask
trask reviewed Sep 11, 2025
View reviewed changes
...g/src/main/java/io/opentelemetry/javaagent/tooling/resources/ResourceCustomizerProvider.java Outdated
// (DistroComponentProvider in this package)
// service: adds "service.name" and "service.instance.id" attributes
// (https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ServiceResourceDetector.java)
private static final List<String> REQUIRED_DETECTORS = Arrays.asList("distribution", "service");
Copy link
Member

Choose a reason for hiding this comment

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

should we leave out service since it's promoting using env var instead of:

https://github.com/open-telemetry/opentelemetry-configuration/blob/b1197246d98be051ede286ea29f20562b062d5e0/examples/kitchen-sink.yaml#L788-L789

Copy link
Member Author

Choose a reason for hiding this comment

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

the follow up: #14639

trask
trask approved these changes Sep 11, 2025
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/14222/files#r2341481892 let's remove service detector for now and we can discuss adding it further (easier to add it later than to remove it later)

trask
trask reviewed Sep 11, 2025
View reviewed changes
...oling/src/main/java/io/opentelemetry/javaagent/tooling/resources/DistroResourceProvider.java
@Override
public Resource createResource(ConfigProperties config) {
return get();
return get("opentelemetry-java-instrumentation");
Copy link
Member

Choose a reason for hiding this comment

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

can you open an issue to change this in 3.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

#14634

@trask trask merged commit 2c3eabf into open-telemetry:main Sep 11, 2025
89 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Declarative Configuration: Java Sep 11, 2025
@zeitlinger zeitlinger deleted the declarative-config-resource-providers branch September 12, 2025 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trask trask trask approved these changes

@laurit laurit laurit left review comments

@jaydeluca jaydeluca jaydeluca approved these changes

Assignees

@zeitlinger zeitlinger

Labels
None yet
Projects
Declarative Configuration: Java
Status: Done
Milestone
v2.20.0
Development

Successfully merging this pull request may close these issues.

missing resource providers for declarative config
4 participants
@zeitlinger @trask @laurit @jaydeluca