-
Notifications
You must be signed in to change notification settings - Fork 996
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.
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
MeterFilters configured after a Meter has been registered #4920
Comments
Hi, I have updated to 1.13.0, everything is ok, but I'm having this debug log popping up. I'm using Vert.x with Guice and I am configuring some defaults meters before creating Vert.x object stacktrace0:54:09.416 DEBUG [main @coroutine#21 ] s.PrometheusMeterRegistry -- A MeterFilter is being configured after a Meter has been registered to this registry. All MeterFilters should be configured before any Meters are registered. If that is not possible or you have a use case where it should be allowed, let the Micrometer maintainers know at https://github.com/micrometer-metrics/micrometer/issues/4920.
java.base/java.lang.Thread.getStackTrace(Thread.java:2450)
at io.micrometer.core.instrument.MeterRegistry$Config.logWarningAboutLateFilter(MeterRegistry.java:844)
at io.micrometer.core.instrument.MeterRegistry$Config.meterFilter(MeterRegistry.java:830)
at io.vertx.micrometer.backends.BackendRegistries.registerMatchers(BackendRegistries.java:127)
at io.vertx.micrometer.backends.BackendRegistries.lambda$setupBackend$0(BackendRegistries.java:82)
at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1708)
at io.vertx.micrometer.backends.BackendRegistries.setupBackend(BackendRegistries.java:62)
at io.vertx.micrometer.impl.VertxMetricsFactoryImpl.metrics(VertxMetricsFactoryImpl.java:54)
at io.vertx.core.spi.VertxMetricsFactory.init(VertxMetricsFactory.java:50)
at io.vertx.core.impl.VertxBuilder.initProviders(VertxBuilder.java:284)
at io.vertx.core.impl.VertxBuilder.init(VertxBuilder.java:275)
at io.vertx.core.Vertx$1.internalBuilder(Vertx.java:121)
at io.vertx.core.Vertx$1.build(Vertx.java:125)
at io.vertx.core.Vertx.vertx(Vertx.java:150)
at VertxModule.vertx(VertxModule.kt:22) code @Provides
@Singleton
fun prometheusMeterRegistry(): PrometheusMeterRegistry {
fun registerDefaultJvmMetrics(registry: PrometheusMeterRegistry) {
JvmGcMetrics().bindTo(registry)
JvmHeapPressureMetrics().bindTo(registry)
JvmInfoMetrics().bindTo(registry)
JvmMemoryMetrics().bindTo(registry)
JvmThreadMetrics().bindTo(registry)
Log4j2Metrics().bindTo(registry)
ProcessorMetrics().bindTo(registry)
UptimeMetrics().bindTo(registry)
}
fun registerNettyAllocatorMetrics(registry: PrometheusMeterRegistry) {
val nettyPooled = PooledByteBufAllocator.DEFAULT
val nettyUnpooled = UnpooledByteBufAllocator.DEFAULT
val vertxPooled = VertxByteBufAllocator.POOLED_ALLOCATOR as PooledByteBufAllocator
val vertxUnpooled = VertxByteBufAllocator.UNPOOLED_ALLOCATOR as UnpooledByteBufAllocator
val nettyName = "netty"
val vertxName = "vertx"
NettyAllocatorBinder
.bindPooled(nettyName, nettyPooled.metric(), registry)
NettyAllocatorBinder
.bindPooled(vertxName, vertxPooled.metric(), registry)
NettyAllocatorBinder
.bindUnpooled(nettyName, nettyUnpooled.metric(), registry)
NettyAllocatorBinder
.bindUnpooled(vertxName, vertxUnpooled.metric(), registry)
}
val registry = PrometheusMeterRegistry(PrometheusConfig.DEFAULT)
registerDefaultJvmMetrics(registry) // <--- bind here
registerNettyAllocatorMetrics(registry) // <--- bind here
return registry
} class VertxModule : AbstractModule() {
@Provides
@Singleton
fun vertx(vertxOptions: VertxOptions, customizer: ObjectMapperCustomizer): Vertx {
val vertx = Vertx.vertx(vertxOptions) // <--- create vert.x object here
return vertx
}
@Provides
@Singleton
fun vertxOptions(
config: ApplicationConfig,
tracer: Tracer,
propagator: TextMapPropagator,
meterRegistry: PrometheusMeterRegistry,
httpClientTagProvider: HttpClientTagProvider,
): VertxOptions = VertxOptions().apply {
// ...
metricsOptions = MicrometerMetricsOptions().apply {
isEnabled = true
micrometerRegistry = meterRegistry // <--- provide registry here
clientRequestTagsProvider = httpClientTagProvider
}
tracingOptions = OpenTelemetryOptions(tracer, propagator)
}
}
public static void bindUnpooled(
@NotNull String name,
@NotNull ByteBufAllocatorMetric metric,
@NotNull MeterRegistry registry
) {
Tags unpooled = Tags.of(NAME, name, ALLOC, UNPOOLED);
memoryUsage(metric, registry, unpooled);
}
private static void memoryUsage(
@NotNull ByteBufAllocatorMetric metric,
@NotNull MeterRegistry registry,
@NotNull Tags tags
) {
Gauge.builder(dot(NETTY, ALLOC, MEMORY, USED), metric, ByteBufAllocatorMetric::usedHeapMemory)
.description("The number of the bytes of the heap memory")
.tags(tags.and(MEMORY, HEAP))
.register(registry);
Gauge.builder(dot(NETTY, ALLOC, MEMORY, USED), metric, ByteBufAllocatorMetric::usedDirectMemory)
.description("The number of the bytes of the direct memory")
.tags(tags.and(MEMORY, DIRECT))
.register(registry);
} |
As the warning and #4917 say, all Is there any way to register them at a later point in the lifecycle? |
Guice doesn't have lifecycle support. You can cheat it by registering default binders after injector creation (or by creating a fake bean): val injector = ...
val registry = injector.getInstance<PrometheusMeterRegistry>()
bindDefaultMetrics(registry) and by adding @Provides
@Singleton
fun foo(fake: Vertx, registry: PrometheusMeterRegistry): Foo = ... It's not really a common case, I guess (using Vert.x with Guice). The message asked if there is a use case, so I provided one. |
I started to get this warning:
I am using micro meters in a Ktor server. Not sure if the issue should be reported here or in the Ktor repository. The way I use it is as next:
|
We are using a @Configuration
public class HealthMetricConfiguration {
@Bean
MeterRegistryCustomizer<MeterRegistry> healthRegistryCustomizer(HealthContributorRegistry healthRegistry) {
final Set<Status> healthList = healthRegistry.stream()
.map(r -> ((HealthIndicator) r.getContributor()).getHealth(false).getStatus())
.collect(Collectors.toSet());
final Status healthStatus = new SimpleStatusAggregator().getAggregateStatus(healthList);
return registry -> registry.gauge(
"app.metrics.actuator.health",
emptyList(),
healthRegistry,
health -> "UP".equals(healthStatus.getCode()) ? 1 : 0
);
}
} |
We use it in a similar way as described by @Ehehal |
Seems Ktor installs a gauge before registering a filter: https://github.com/ktorio/ktor/blob/1bc44e0715d4b7ffd9669a03a3fe02314c0b11b6/ktor-server/ktor-server-plugins/ktor-server-metrics-micrometer/jvm/src/io/ktor/server/metrics/micrometer/MicrometerMetrics.kt#L121 |
Hi there,
|
When using SpringBoot, MeterFilters are applied via the MeterRegistryPostProcessor. This implementation first applies customizations to the MeterRegistry via MeterRegistryCustomizers. Today, we have a customizer that is adding gauges to the MeterRegistry, which is why the log message started to show up. I switched this to using a MeterBinder to remove the log messages. |
But whatever creates the registry, it should be able to gather all the filters and all the binders and register the filters first then the binders, right?
I can you (or ask Ktor to do this for you) register the
I don't think you should do this from a
Could you please open an issue for Ktor to register the gauge from a
I think registering a |
I fully agree. We don't do this for every meter. However we want to give the developers the freedom to define meters with a different set of percentiles especially when it comes to validate/verify new features since we write code for a high volume/low latency product. For example our standard meter filter set the publishPercentileHistogram to false and percentiles to 0.95 and 0.99. However, our developers should have the ability to enable publishPercentileHistogram or define different percentiles on a case to case basis. A possible solution for our problem could be to define a set of standard meter filters for all use cases we encounter and to find a way to address these when creating a new meter. I assume this should be possible with adding a |
I'm not sure about that.
step 2/3 are out of my control, see the stacktrace:
|
@jonatan-ivanov Aha thanks.
to:
which seems to remove the warning |
Instead of what you are doing now I think there should be alternatives:
I get that but that's not an issue with Guice but seemingly a bug in the instrumentation of Vert.x since it seems it creates meters and registers filters after that. I would try to open an issue there. @thyzzv 👍🏼 |
We have a usecase at Omnissa where we are integrating with Conductor
So we are letting the devs here know that we have this usecase. |
@wildMythicWest I'm not sure I follow. You should do what the log message tells you: register the |
Now that you mention it. The config is applied on AfterPropertiesSet, Maybe if I change this the message will dissapear. I will check. |
I want to understand what the negative effects are. Can you explain in more detail? |
@MRuecker great question. I should have elaborated in the original description. The description of #4917 has a bit more:
Maybe that still sounds a bit abstract. More concretely, looking at some test code: @Test
void meterRegistrationBeforeAndAfterMeterFilterRegistration() {
Counter c1 = registry.counter("counter");
registry.config().meterFilter(MeterFilter.commonTags(Tags.of("common", "tag")));
Counter c2 = registry.counter("counter");
} It's perhaps more obvious written this way that the Meter referenced by public class MyComponent {
private final MeterRegistry meterRegistry;
private final Counter invocations;
public MyComponent(MeterRegistry meterRegistry) {
this.meterRegistry = meterRegistry;
this.invocations = meterRegistry.counter("mycomponent.invocations");
}
public void doSomething() {
this.invocations.increment();
// ...
}
} If you want to configure a common tag for all your metrics, whether it will apply to the counter in this component depends on whether this component is instantiated before or after the common tag is configured. On the other hand, consider the following different implementation. If the counter is retrieved each time (because, say, it has a dynamic tag), then it may not have the common tag for some number of increments until the common tag is configured, but subsequent increments will be incrementing the meter with the common tag since the meter is being retrieved (or created) each time. In this way, the negative effects (filters are not applied to meters registered before the filters are configured) are minimized because the meter is being retrieved each time. public class MyComponent2 {
private final MeterRegistry meterRegistry;
public MyComponent2(MeterRegistry meterRegistry) {
this.meterRegistry = meterRegistry;
}
public void doSomething(String type) {
this.meterRegistry.counter("mycomponent2.invocations", "type", sanitizeAndNormalize(type)).increment();
// ...
}
} The meter registered before the filter was configured will still be in the registry and is in a sense a leak. Its increments won't be part of the count in the counter with the common tag. Does that help explain things? |
Thank you for the detailled explanation. This makes more sense and I see where you are coming from. Our use case is a bit different. When we create a new Meter with a specific MeterFilter configuration we make sure that this filter applies only to the new Meter. This is handled by the last method in my example I posted earlier:
This filter will only apply to the Meter that has been created which is guarded by the statement
With this we make sure that the new MeterFilter is not accidently applied to other Meters |
@MRuecker what you've described sounds exactly like the kind of support a framework usually provides. I'm not sure if you're using a framework, but for example, see Spring Boot's docs on per-meter properties which allow specifying the distribution statistics config for all meters that start with a name. This translates into a |
We use various frameworks for our (roughly 150) microservices, mostly Vertx, Micronaut and Ktor. |
That shouldn't generally be a problem. In the example code I showed, there's nothing framework specific. What the framework should be doing is ensuring that the |
I get you point and for most of our meters we use existing MeterFilters that have been created in our framework wrapper before any Meter creation. However sometimes developers want to create meters with a specific configuration in their lib (mostly temporary) to get a better picture what's going on in their code - and I'm trying to find a solution for this use case. |
The solution generally is to follow the documentation for the Micrometer integration in the framework being used to configure your own MeterFilter. I think the challenge, if I'm understanding correctly, is that the exact answer is a bit different depending on which framework is used. There may be an opportunity to document these kinds of features that framework integrations should have (configuring a custom MeterFilter on the registry the framework creates/uses) and working with the various frameworks to make sure it's easy for users to understand how to do that. As an example of what's available today for a developer working on an app that uses Micronaut, they can follow this part of the documentation to configure a custom |
We decided to load all MeterFilters via java ServiceLoader when configuring the meter registry in our various frameworks. |
fyi, here are the connected issues so far: |
@jonatan-ivanov I moved the Spectator-to-micrometer bridge to the bean initialization phase, but the warning is still here. Here is the setup code: @Configuration
public class MicrometerConfig {
private final MicrometerProperties micrometerProperties;
public MicrometerConfig(
final MicrometerProperties micrometerProperties, final Environment environment) {
this.micrometerProperties = micrometerProperties;
micrometerProperties.getConfiguredMetricModes().stream()
.map(CommonMetricsMode::resolve)
.map(t -> t.allocateMeterRegistry(environment))
.forEach(
reg -> {
configureFilters(reg); // <- reg is a io.micrometer.core.instrument.MeterRegistry
MicrometerRegistry registry = new MicrometerRegistry(reg); // MicrometerRegistry is com.netflix.spectator.micrometer.MicrometerRegistry
Spectator.globalRegistry().add(registry);
});
}
private void configureFilters(MeterRegistry reg) {
... add meter filters logic
}
}
|
@wildMythicWest
I think doing the above should be fairly easy as it seems you are using Spring Boot but what you are doing in your example (doing everything in the ctor of a
@Configuration
public class MicrometerConfig {
@Bean
MeterFilter customMeterFilter() {
return ...
}
// or @Bean MeterRegistryCustomizer<...> ...
}
@Configuration
public class SpectatorConfig {
// I would not do what you did in the first place (global registry)
// but if I need to, I would do it in the @PostConstruct method of a "registrar" @Component or @Configuration
} (Btw maybe you can do this whole thing in a single |
Quarkus will need to register MeterFilters before Meters. There are multiple reasons for this to happen, one of them is how other extensions use Micrometer. There are initialisation precedences to the initialisation of Micrometer extension itself, namely, Vert.x requires very early that the registry is present. This creates a chicken and egg init problem that is currently handled by the static init from above. |
In #4917 we took a step toward warning users about this situation with the following log message:
Historically, we have allowed configuring MeterFilters after meters have been registered. Depending on how meters are used, the negative effects of doing this can be minimal. It is ideal to configure all desired MeterFilters before registering any Meters so that all registered Meters will have all configured MeterFilters applied.
If users have a use case for configuring MeterFilters "late" (after a Meter has been registered), we would be interested in understanding that so we can make informed decisions about what we allow in the future. It may also be the case that something out of a user's direct control is causing this situation to happen - a framework or library the user is using may be configuring MeterFilters or registering Meters at timing not controlled by the user. We would like to hear about these cases so we can work with the libraries or frameworks. The above log message includes a stack trace when DEBUG level logging is enabled on the meter registry implementation in question. This stack trace will help track down what is configuring the MeterFilter late.
The text was updated successfully, but these errors were encountered: