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
Merged

Added resources as runtimeOnly dependency #14

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
029b69a
Added resources as runtimeOnly dependency
kjrun316 May 3, 2023
055d056
Updated getEndpoint logging since default onprem endpoints are possible
kjrun316 May 4, 2023
6853fdc
Add instrumentation resources
kjrun316 May 5, 2023
463c7ef
Updated comments for globalAttributes prop
kjrun316 May 5, 2023
3475fa3
Merge branch 'main' into Add_instrumentation_resources
zeitlinger May 5, 2023
25292ba
Update src/main/java/com/grafana/opentelemetry/GrafanaProperties.java
kjrun316 May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.gradle/
build/
.env
.idea/
1 change: 1 addition & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

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 •
edited
Loading

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.

implementation "io.opentelemetry.instrumentation:opentelemetry-micrometer-1.5:$otelVersion-alpha"
implementation "io.opentelemetry:opentelemetry-sdk-extension-autoconfigure:$otelVersion-alpha"
runtimeOnly 'io.micrometer:micrometer-tracing-bridge-otel'
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/com/grafana/opentelemetry/OpenTelemetryConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,15 @@ static Optional<String> getEndpoint(String endpoint, String zone, Optional<Strin
}
} else {
if (hasZone) {
logger.warn("ignoring grafana.otlp.cloud.zone, because grafana.otlp.onprem.endpoint was found");
logger.warn("ignoring grafana.otlp.cloud.zone, because grafana.otlp.cloud.instanceId was not found");
zeitlinger marked this conversation as resolved.
Show resolved Hide resolved
}
if (hasEndpoint) {
return Optional.of(endpoint);
} else {
logger.warn("please specify grafana.otlp.onprem.endpoint");
logger.info("grafana.otlp.onprem.endpoint not found, using default enpoint for otel.exporter.otlp.protocol");
}


}
return Optional.empty();
}
Expand Down
19 changes: 12 additions & 7 deletions src/test/java/com/grafana/opentelemetry/OpenTelemetryConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,13 @@ private static Stream<Arguments> basicAuthCases() {

@ParameterizedTest(name = "{0}")
@MethodSource("endpointCases")
void getEndpoint(String name, Optional<String> expected, String expectedOutput,
String zone, String endpoint, Optional<String> authHeader, CapturedOutput output) {
void getEndpoint(String name,
Optional<String> expected,
String expectedOutput,
String zone,
String endpoint,
Optional<String> authHeader,
CapturedOutput output) {
Assertions.assertThat(OpenTelemetryConfig.getEndpoint(endpoint, zone, authHeader)).isEqualTo(expected);
Assertions.assertThat(output).contains(expectedOutput);
}
Expand All @@ -85,24 +90,24 @@ private static Stream<Arguments> endpointCases() {
Arguments.of("only zone",
Optional.of("https://otlp-gateway-zone.grafana.net/otlp"), "",
"zone", "", Optional.of("apiKey")),
Arguments.of("only endpoint",
Arguments.of("only onprem endpoint",
Optional.of("endpoint"), "",
"", "endpoint", Optional.empty()),
Arguments.of("both with cloud",
Optional.of("https://otlp-gateway-zone.grafana.net/otlp"),
"ignoring grafana.otlp.onprem.endpoint, because grafana.otlp.cloud.instanceId was found",
"zone", "endpoint", Optional.of("key")),
Arguments.of("both without cloud",
Arguments.of("zone without instanceId",
Optional.of("endpoint"),
"ignoring grafana.otlp.cloud.zone, because grafana.otlp.onprem.endpoint was found",
"ignoring grafana.otlp.cloud.zone, because grafana.otlp.cloud.instanceId was not found",
"zone", "endpoint", Optional.empty()),
Arguments.of("missing zone",
Optional.empty(),
"please specify grafana.otlp.cloud.zone",
" ", " ", Optional.of("key")),
Arguments.of("missing endpoint",
Arguments.of("onprem endpoint not set",
Optional.empty(),
"please specify grafana.otlp.onprem.endpoint",
"grafana.otlp.onprem.endpoint not found, using default enpoint for otel.exporter.otlp.protocol",
" ", " ", Optional.empty())
);
}
Expand Down