Skip to content
This repository has been archived by the owner on Jun 24, 2024. It is now read-only.

Added resources as runtimeOnly dependency #14

Merged
merged 6 commits into from
May 9, 2023

Conversation

kjrun316
Copy link
Contributor

@kjrun316 kjrun316 commented May 3, 2023

No description provided.

@CLAassistant
Copy link

CLAassistant commented May 3, 2023

CLA assistant check
All committers have signed the CLA.

@kjrun316
Copy link
Contributor Author

kjrun316 commented May 3, 2023

Note: adding the instrumentation-resources jar will mean that applications incorporating the spring-starter will by default log all attributes associated with the predefined ResourceProviders. Users can enable/diasable ResourceProviders using the following properties. (Refer here for more details)

- otel.java.enabled.resource.providers
- otel.java.disabled.resource.providers

Our instrumentation docs will be updated to reflect this change.

@@ -40,6 +40,7 @@ dependencies {
implementation "io.opentelemetry:opentelemetry-exporter-otlp-logs:$otelVersion-alpha"
implementation "io.opentelemetry:opentelemetry-exporter-logging" // only for debug
runtimeOnly("io.opentelemetry.instrumentation:opentelemetry-logback-appender-1.0:$otelVersion-alpha")
runtimeOnly "io.opentelemetry.instrumentation:opentelemetry-resources:$otelVersion-alpha"
Copy link
Member

Choose a reason for hiding this comment

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

great finding - didn't know about that!

Just checked out the implementation, and we should probably also use InetAddress.getLocalHost().getHostName() instead of System.getenv("HOSTNAME") as here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.

@@ -40,6 +40,7 @@ dependencies {
implementation "io.opentelemetry:opentelemetry-exporter-otlp-logs:$otelVersion-alpha"
implementation "io.opentelemetry:opentelemetry-exporter-logging" // only for debug
runtimeOnly("io.opentelemetry.instrumentation:opentelemetry-logback-appender-1.0:$otelVersion-alpha")
runtimeOnly "io.opentelemetry.instrumentation:opentelemetry-resources:$otelVersion-alpha"
Copy link
Member

Choose a reason for hiding this comment

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

The service name is also detected here:

I think the manifest should have priority over the jar name - can you check if that's the case?

Copy link
Member

Choose a reason for hiding this comment

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

might also make sense to describe how the service.name is created from both the manifest and the jar name there.

Copy link
Contributor Author

@kjrun316 kjrun316 May 4, 2023

Choose a reason for hiding this comment

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

Took a closer look. Below is the order of precedence for service name

  1. OTEL_SERVICE_NAME
  2. OTEL_RESOURCE_ATTRIBUTES
    a. service_name key:value (manually set)
    b. spring application name
    c. manifest

If none of the above are set the service_name shows as unknown_service:java

Note that JarServiceNameDetector extends ConditionalResourceProvider so it will only be applied if the shouldApply method returns true.

- Updated method call for hostname to match call in Otel's HostResource
- Fixed typo in log message
@kjrun316 kjrun316 merged commit 9947067 into main May 9, 2023
2 checks passed
@kjrun316 kjrun316 deleted the Add_instrumentation_resources branch May 9, 2023 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants