Skip to content
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

Spring Boot Starter service-name is constant #5359

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Spring Boot Starter service-name is constant #5359

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53678f2
Spring Boot Starter service-name is constant
aschugunov Feb 11, 2022
393e0b9
Add ResourceProvider beans for spring with ConfigProperties
aschugunov Mar 29, 2022
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
20 changes: 20 additions & 0 deletions instrumentation/spring/spring-boot-autoconfigure/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,26 @@ If an exporter is present in the classpath during runtime and a spring bean of t

<!-- Slf4j Log Correlation otel.springboot.loggers.slf4j.enabled true org.slf4j.MDC -->

##### Resource Properties

| Feature | Property | Default Value |
|----------|--------------------------------------------------|------------------------|
| Resource | otel.springboot.resource.enabled | `true` |
| | otel.springboot.resource.attributes.service.name | `unknown_service:java` |
| | otel.springboot.resource.attributes | `empty map` |

`unknown_service:java` will be used as the service-name if no value has been specified to the
property `spring.application.name` or `otel.springboot.resource.attributes.service.name` (which has
the highest priority)

`otel.springboot.resource.attributes` supports a pattern-based resource configuration in the
application.properties like this:

```
otel.springboot.resource.attributes.environment=dev
otel.springboot.resource.attributes.xyz=foo
```

##### Exporter Properties

| Feature | Property | Default Value |
Expand Down
2 changes: 2 additions & 0 deletions instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies {
compileOnly("io.opentelemetry:opentelemetry-extension-annotations")
compileOnly("io.opentelemetry:opentelemetry-extension-trace-propagators")
compileOnly("io.opentelemetry:opentelemetry-extension-aws")
compileOnly("io.opentelemetry:opentelemetry-sdk-extension-resources")
compileOnly("io.opentelemetry:opentelemetry-exporter-logging")
compileOnly("io.opentelemetry:opentelemetry-exporter-jaeger")
compileOnly("io.opentelemetry:opentelemetry-exporter-otlp")
Expand All @@ -40,6 +41,7 @@ dependencies {
testImplementation(project(":testing-common"))
testImplementation("io.opentelemetry:opentelemetry-sdk")
testImplementation("io.opentelemetry:opentelemetry-sdk-testing")
testImplementation("io.opentelemetry:opentelemetry-sdk-extension-resources")
testImplementation("io.opentelemetry:opentelemetry-extension-annotations")
testImplementation("io.opentelemetry:opentelemetry-extension-trace-propagators")
testImplementation("io.opentelemetry:opentelemetry-extension-aws")
Expand Down
31 changes: 30 additions & 1 deletion ...io/opentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,26 @@
package io.opentelemetry.instrumentation.spring.autoconfigure;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
import java.util.Collections;
import java.util.List;
import java.util.function.Supplier;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.Environment;

/**
* Create {@link io.opentelemetry.api.trace.Tracer} bean if bean is missing.
Expand All @@ -41,18 +46,42 @@ public static class OpenTelemetryBeanConfig {
@ConditionalOnMissingBean
public SdkTracerProvider sdkTracerProvider(
SamplerProperties samplerProperties,
ObjectProvider<List<SpanExporter>> spanExportersProvider) {
ObjectProvider<List<SpanExporter>> spanExportersProvider,
Resource otelResource) {
SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();

spanExportersProvider.getIfAvailable(Collections::emptyList).stream()
.map(spanExporter -> BatchSpanProcessor.builder(spanExporter).build())
.forEach(tracerProviderBuilder::addSpanProcessor);

return tracerProviderBuilder
.setResource(otelResource)
.setSampler(Sampler.traceIdRatioBased(samplerProperties.getProbability()))
.build();
}

@Bean
@ConditionalOnMissingBean
public Resource otelResource(
Environment env, ObjectProvider<List<Supplier<Resource>>> resourceProviders) {
String applicationName = env.getProperty("spring.application.name");
Resource resource = defaultResource(applicationName);
List<Supplier<Resource>> resourceCustomizers =
resourceProviders.getIfAvailable(Collections::emptyList);
for (Supplier<Resource> resourceCustomizer : resourceCustomizers) {
resource = resource.merge(resourceCustomizer.get());
}
return resource;
}

private static Resource defaultResource(String applicationName) {
if (applicationName == null) {
return Resource.getDefault();
}
return Resource.getDefault()
.merge(Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, applicationName)));
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍


@Bean
public OpenTelemetry openTelemetry(
ObjectProvider<ContextPropagators> propagatorsProvider, SdkTracerProvider tracerProvider) {
Expand Down
72 changes: 72 additions & 0 deletions .../io/opentelemetry/instrumentation/spring/autoconfigure/OtelResourceAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.sdk.extension.resources.ContainerResource;
import io.opentelemetry.sdk.extension.resources.HostResource;
import io.opentelemetry.sdk.extension.resources.OsResource;
import io.opentelemetry.sdk.extension.resources.ProcessResource;
import io.opentelemetry.sdk.extension.resources.ProcessRuntimeResource;
import io.opentelemetry.sdk.resources.Resource;
import java.util.Map;
import java.util.function.Supplier;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
@EnableConfigurationProperties(OtelResourceProperties.class)
@AutoConfigureBefore(OpenTelemetryAutoConfiguration.class)
@ConditionalOnProperty(prefix = "otel.springboot.resource", name = "enabled", matchIfMissing = true)
public class OtelResourceAutoConfiguration {

@Bean
public Supplier<Resource> otelResourceProvider(OtelResourceProperties otelResourceProperties) {
return () -> {
AttributesBuilder attributesBuilder = Attributes.builder();
for (Map.Entry<String, String> entry : otelResourceProperties.getAttributes().entrySet()) {
attributesBuilder.put(entry.getKey(), entry.getValue());
}
Attributes attributes = attributesBuilder.build();
return Resource.create(attributes);
};
}

@Bean
@ConditionalOnClass(OsResource.class)
public Supplier<Resource> otelOsResourceProvider() {
return OsResource::get;
}
Copy link
Member

Choose a reason for hiding this comment

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

what do you think of loading the resources via SPI? https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/resources/src/main/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider

or are there advantages to having them as separate named beans?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I dont deeply understand how SPI works.
Currently, trace does not contain any inforamtion about os, container and other resources.
Main idea is combine all resources via supplier resource beans and then inject complex resource into tracerProvider.
maybe it is possbile to do with SPI, I will try to research. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to load resource objects via spring.factories and it seems spring does not load bean by interface or I did it in wrong manner.
If I understand correctly java loads objects via SPI when implementation is located in classpath, spring can customize context via spring.factories file. But this way does not work

io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider=\
io.opentelemetry.sdk.extension.resources.ContainerResourceProvider,\
io.opentelemetry.sdk.extension.resources.HostResourceProvider,\
io.opentelemetry.sdk.extension.resources.OsResourceProvider,\
io.opentelemetry.sdk.extension.resources.ProcessResourceProvider,\
io.opentelemetry.sdk.extension.resources.ProcessRuntimeResourceProvider

I added @ConditionalOnClass over appropriate bean, maybe it will be more suitable

Copy link
Member

Choose a reason for hiding this comment

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

How about registering the ResourceProviders as beans? That's the same interface that OTel SDK autoconfigure uses - but instead of using SPI/ServiceLoader you'd just load all beans of that type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you explain a bit more, please?
Did you mean to create resourceProvider beans in addition to resource beans?

Copy link
Member

Choose a reason for hiding this comment

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

So, instead of all the Supplier<Resource> beans in this class, you could use ResourceProvider instead:

  @Bean
  @ConditionalOnClass(OsResource.class)
  public ResourceProvider osResourceProvider() {
    return new OsResourceProvider();
  }

  @Bean
  @ConditionalOnClass(ProcessResource.class)
  public ResourceProvider processResourceProvider() {
    return new ProcessResourceProvider();
  }

 // ...

Then, in the OpenTelemetryAutoConfiguration class you'd change the otelResource method so that it uses these beans:

    @Bean
    @ConditionalOnMissingBean
    public Resource otelResource(
        Environment env, ObjectProvider<List<ResourceProvider>> resourceProviders) {
      String applicationName = env.getProperty("spring.application.name");
      Resource resource = defaultResource(applicationName);
      ConfigProperties config = SpringConfigProperties(env);
      for (ResourceProvider provider : resourceProviders.getIfAvailable(Collections::emptyList)) {
        resource = resource.merge(provider.createResource(config));
      }
      return resource;
    }

ResourceProvider#createResource requires a ConfigProperties argument, but it would be quite simple to implement one that just simply delegates to Spring's Environment class.

This way you could reuse the SDK ResourceProvider interface in a way that's compatible with Spring beans -- instead of using SPI, the user of the instrumented application could just configure a ResourceProvider bean in their app and it'd get picked up automatically by this lib.

WDYT about that?

Copy link
Contributor Author

@aschugunov aschugunov Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes it is a great idea to reuse existed interface for getting all available resources.
I see benefit of interface, it can be more clear for whole open telemetry project because interface provides consistent concept.
On the other hand, supplier follows the same idea - to provide resource. I mean there is no difference how spring will provide beans - via ResourceProvider interface or via Supplier<Resource>. Also interface enforce to use ConfigProperties that is not used in any current resourceProviders implementations.
OtelResourceProperties is such type of config. It is created by spring automatically.

If I understand correctly I should implement something like SpringResourceProvider and encapsulate there logic of determination of application name and attributes.

I will add these changes in second commit in this PR in a day or two. So you will be able to evaluate changes and choose more appropriate implementation.

Copy link
Member

Choose a reason for hiding this comment

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

I will add these changes in second commit in this PR in a day or two. So you will be able to evaluate changes and choose more appropriate implementation.

👍 Thanks!


@Bean
@ConditionalOnClass(ProcessResource.class)
public Supplier<Resource> otelProcessResourceProvider() {
return ProcessResource::get;
}

@Bean
@ConditionalOnClass(ProcessRuntimeResource.class)
public Supplier<Resource> otelProcessRuntimeResourceProvider() {
return ProcessRuntimeResource::get;
}

@Bean
@ConditionalOnClass(HostResource.class)
public Supplier<Resource> otelHostResourceProvider() {
return HostResource::get;
}

@Bean
@ConditionalOnClass(ContainerResource.class)
public Supplier<Resource> otelContainerResource() {
return ContainerResource::get;
}
}
23 changes: 23 additions & 0 deletions ...in/java/io/opentelemetry/instrumentation/spring/autoconfigure/OtelResourceProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure;

import java.util.Collections;
import java.util.Map;
import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "otel.springboot.resource")
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved
public class OtelResourceProperties {
private Map<String, String> attributes = Collections.emptyMap();

public Map<String, String> getAttributes() {
return attributes;
}

public void setAttributes(Map<String, String> attributes) {
this.attributes = attributes;
}
}
3 changes: 2 additions & 1 deletion ...rumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/spring.factories
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfigura
io.opentelemetry.instrumentation.spring.autoconfigure.httpclients.resttemplate.RestTemplateAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.httpclients.webclient.WebClientAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.webmvc.WebMvcFilterAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.aspects.TraceAspectAutoConfiguration
io.opentelemetry.instrumentation.spring.autoconfigure.aspects.TraceAspectAutoConfiguration,\
io.opentelemetry.instrumentation.spring.autoconfigure.OtelResourceAutoConfiguration
31 changes: 31 additions & 0 deletions ...pentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@

package io.opentelemetry.instrumentation.spring.autoconfigure;

import static io.opentelemetry.semconv.resource.attributes.ResourceAttributes.SERVICE_NAME;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -69,4 +71,33 @@ void initializeOpenTelemetry() {
.hasBean("customTracerProvider")
.doesNotHaveBean("sdkTracerProvider"));
}

@Test
@DisplayName(
"when spring.application.name is set value should be passed to service name attribute")
void shouldDetermineServiceNameBySpringApplicationName() {
this.contextRunner
.withPropertyValues("spring.application.name=myapp-backend")
.withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class))
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);

assertThat(otelResource.getAttribute(SERVICE_NAME)).isEqualTo("myapp-backend");
});
}

@Test
@DisplayName(
"when spring application name and otel service name are not set service name should be default")
void hasDefaultServiceName() {
this.contextRunner
.withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class))
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);

assertThat(otelResource.getAttribute(SERVICE_NAME)).isEqualTo("unknown_service:java");
});
}
}
82 changes: 82 additions & 0 deletions ...opentelemetry/instrumentation/spring/autoconfigure/OtelResourceAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure;

import static io.opentelemetry.semconv.resource.attributes.ResourceAttributes.SERVICE_NAME;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.sdk.resources.Resource;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;

public class OtelResourceAutoConfigurationTest {
private final ApplicationContextRunner contextRunner =
new ApplicationContextRunner()
.withConfiguration(
AutoConfigurations.of(
OtelResourceAutoConfiguration.class, OpenTelemetryAutoConfiguration.class));

@Test
@DisplayName("when otel service name is set it should be set as service name attribute")
void shouldDetermineServiceNameByOtelServiceName() {
this.contextRunner
.withPropertyValues(
"otel.springboot.resource.attributes.service.name=otel-name-backend",
"otel.springboot.resource.enabled=true")
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);

assertThat(otelResource.getAttribute(SERVICE_NAME)).isEqualTo("otel-name-backend");
});
}

@Test
@DisplayName(
"when otel.springboot.resource.enabled is not specified configuration should be initialized")
void shouldInitAutoConfigurationByDefault() {
this.contextRunner
.withPropertyValues("otel.springboot.resource.attributes.service.name=otel-name-backend")
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);

assertThat(otelResource.getAttribute(SERVICE_NAME)).isEqualTo("otel-name-backend");
});
}

@Test
@DisplayName(
"when otel.springboot.resource.enabled is set to false configuration should NOT be initialized")
void shouldNotInitAutoConfiguration() {
this.contextRunner
.withPropertyValues(
"otel.springboot.resource.attributes.service.name=otel-name-backend",
"otel.springboot.resource.enabled=false")
.run(context -> assertThat(context.containsBean("otelResourceProvider")).isFalse());
}

@Test
@DisplayName("when otel attributes are set in properties they should be put in resource")
void shouldInitializeAttributes() {
this.contextRunner
.withPropertyValues(
"otel.springboot.resource.attributes.xyz=foo",
"otel.springboot.resource.attributes.environment=dev",
"otel.springboot.resource.enabled=true")
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);

assertThat(otelResource.getAttribute(AttributeKey.stringKey("environment")))
.isEqualTo("dev");
assertThat(otelResource.getAttribute(AttributeKey.stringKey("xyz"))).isEqualTo("foo");
});
}
}
Loading