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

fix micrometer bridge auto configuration annotation #13083

Merged
merged 16 commits into from
Feb 12, 2025
Merged

fix micrometer bridge auto configuration annotation #13083

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
293532b
fix annotation
zeitlinger Jan 22, 2025
b16bd81
fix annotation
zeitlinger Jan 23, 2025
ebb1ec1
fix annotation
zeitlinger Jan 23, 2025
d9dd0c0
fix annotation
zeitlinger Jan 27, 2025
e1da70e
fix annotation
zeitlinger Jan 27, 2025
a5b122b
improve test
zeitlinger Jan 28, 2025
84abdcc
sort the otel registry last explicitly
zeitlinger Feb 6, 2025
e0e55bf
fix dependency
zeitlinger Feb 7, 2025
ad052db
test order
zeitlinger Feb 7, 2025
0061864
fix test
zeitlinger Feb 7, 2025
57e3ef9
format
zeitlinger Feb 7, 2025
5d67753
running in latest dep doesn't work
zeitlinger Feb 7, 2025
3c5d708
including prometheus conditionally doesn't seem to work - and it not …
zeitlinger Feb 7, 2025
01acb0d
exclude auto config
zeitlinger Feb 7, 2025
b9ae5d1
use testLibrary
zeitlinger Feb 10, 2025
c24dcb2
Update instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/…
zeitlinger Feb 12, 2025
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
16 changes: 16 additions & 0 deletions instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/javaagent/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ dependencies {
latestDepTestLibrary("ch.qos.logback:logback-classic:+")
}

testing {
suites {
val testPrometheus by registering(JvmTestSuite::class) {
dependencies {
runtimeOnly("io.micrometer:micrometer-registry-prometheus:1.14.3")
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

since there are no source for this suite it won't run any tests as far as I can tell

Copy link
Member Author

Choose a reason for hiding this comment

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

OK - I had to test this by adding id("com.adarshr.test-logger") version "4.0.0" to see that noting gets executed...

used the setup for stable semconv now


tasks.withType<Test>().configureEach {
// required on jdk17
jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
Expand Down Expand Up @@ -49,3 +59,9 @@ if (!latestDepTest) {
}
}
}

tasks {
check {
dependsOn(testing.suites)
}
}
34 changes: 34 additions & 0 deletions ...ent/instrumentation/spring/actuator/v2_0/OpenTelemetryMeterRegistryAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@

import io.micrometer.core.instrument.Clock;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
import io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.MicrometerSingletons;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.boot.actuate.autoconfigure.metrics.CompositeMeterRegistryAutoConfiguration;
import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration;
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
Expand All @@ -32,4 +40,30 @@ public class OpenTelemetryMeterRegistryAutoConfiguration {
public MeterRegistry otelMeterRegistry() {
return MicrometerSingletons.meterRegistry();
}

@Bean
static BeanPostProcessor postProcessCompositeMeterRegistry() {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a convention of making these methods public?

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 other bean post processor is also static:

opentelemetry-java-instrumentation/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/JdbcInstrumentationAutoConfiguration.java

Line 34 in 428aa28

static DataSourcePostProcessor dataSourcePostProcessor(

return new BeanPostProcessor() {
@Override
public Object postProcessAfterInitialization(Object bean, String beanName) {
if (bean instanceof CompositeMeterRegistry) {
CompositeMeterRegistry original = (CompositeMeterRegistry) bean;
List<MeterRegistry> list = new ArrayList<>(original.getRegistries());
// sort otel registry last
list.sort(
Comparator.comparingInt(
value -> value == MicrometerSingletons.meterRegistry() ? 1 : 0));
Set<MeterRegistry> registries = new LinkedHashSet<>(list);
return new CompositeMeterRegistry(
original.config().clock(), Collections.singletonList(original)) {
@Override
public Set<MeterRegistry> getRegistries() {
return registries;
}
};
}
return bean;
}
};
}
}
21 changes: 17 additions & 4 deletions ...st/java/io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/ActuatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.javaagent.instrumentation.spring.actuator.v2_0.SpringApp.TestBean;
import java.util.Collection;
import org.assertj.core.api.AbstractCollectionAssert;
import org.assertj.core.api.ObjectAssert;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.springframework.boot.SpringApplication;
Expand Down Expand Up @@ -57,9 +60,19 @@ void shouldInjectOtelMeterRegistry() {
"value"))))));

MeterRegistry meterRegistry = context.getBean(MeterRegistry.class);
assertThat(meterRegistry).isNotNull().isInstanceOf(CompositeMeterRegistry.class);
assertThat(((CompositeMeterRegistry) meterRegistry).getRegistries())
.anyMatch(r -> r.getClass().getSimpleName().equals("OpenTelemetryMeterRegistry"))
.anyMatch(r -> r.getClass().getSimpleName().equals("SimpleMeterRegistry"));
assertThat(meterRegistry).isInstanceOf(CompositeMeterRegistry.class);
AbstractCollectionAssert<
?, Collection<? extends MeterRegistry>, MeterRegistry, ObjectAssert<MeterRegistry>>
match =
assertThat(((CompositeMeterRegistry) meterRegistry).getRegistries())
.anyMatch(r -> r.getClass().getSimpleName().equals("OpenTelemetryMeterRegistry"))
.anyMatch(r -> r.getClass().getSimpleName().equals("SimpleMeterRegistry"));

try {
Class.forName("io.micrometer.prometheusmetrics.PrometheusMeterRegistry");
match.anyMatch(r -> r.getClass().getSimpleName().equals("PrometheusMeterRegistry"));
} catch (ClassNotFoundException e) {
// not testing prometheus
}
}
}
Loading