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.

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

Gauges may be silently ignored when MeterFilters drop or transform tags #5616

Closed
keith-turner opened this issue Oct 31, 2024 · 4 comments · Fixed by #5617
Closed

Gauges may be silently ignored when MeterFilters drop or transform tags #5616

keith-turner opened this issue Oct 31, 2024 · 4 comments · Fixed by #5617
Labels
doc-update A documentation update
Milestone

Comments

@keith-turner
Copy link

Describe the bug

As demonstrated by this example program Gauges can be silently ignored when meter filters drop or transform tags.

import io.micrometer.core.instrument.Gauge;
import io.micrometer.core.instrument.config.MeterFilter;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;

import java.util.concurrent.atomic.AtomicLong;

public class Test {

    public static void main(String[] args) throws Exception {

        var registry = new SimpleMeterRegistry();
        registry.config().meterFilter(MeterFilter.ignoreTags("tag1"));

        AtomicLong al1 = new AtomicLong(2);
        AtomicLong al2 = new AtomicLong(3);

        Gauge.builder("test.meter1", al1::get).tag("tag1","val1").tag("tag2","val2").register(registry);
        Gauge.builder("test.meter1", al2::get).tag("tag1","val2").tag("tag2","val2").register(registry);

        System.out.println(registry.getMetersAsString());

        al1.set(7);
        al2.set(11);

        System.out.println(registry.getMetersAsString());
    }
}

When run this example outputs :

test.meter1(GAUGE)[tag2='val2']; value=2.0
test.meter1(GAUGE)[tag2='val2']; value=7.0

The second gauge that was registered and its associated atomic long are silently ignored by the metrics system. Only the data related to the first gauge is seen. Tested with LoggingMeterRegistry and CompositeMeterRegistry also and saw the same behavior.

Environment

  • Micrometer version 1.13.6
  • Micrometer registry CompositeMeterRegistry
  • OS: Linux
  • Java version: openjdk version "17.0.13" 2024-10-15 LTS

Expected behavior

There are a few things that could be done for this. Could simply document the behavior, was not able to find any warning about this in the micrometer documentation. Could add a way for user to specify a function to merge gauges. Could make registration fail with an exception if the gauge would be ignored. Could log a warning that data is being ignored.

Going forward an exception or log message warning that data is being ignored seems best because it would make it obvious to the user that data is not being seen. For the currently released version it would be nice to update the docs.

@jonatan-ivanov
Copy link
Member

Thank you for the issue!
This is happening because the code above effectively does this (I removed the filter and tag1 just to make things simpler):

SimpleMeterRegistry registry = new SimpleMeterRegistry();

AtomicLong al1 = new AtomicLong(2);
AtomicLong al2 = new AtomicLong(3);

Gauge.builder("test.meter1", al1::get).tag("tag2","val2").register(registry);
Gauge.builder("test.meter1", al2::get).tag("tag2","val2").register(registry);

System.out.println(registry.getMetersAsString());
al1.set(7);
al2.set(11);
System.out.println(registry.getMetersAsString());

At the second registration, the Gauge that was registered in the previous line will be returned so the builder will not be applied since the Gauge is already there and the Supplier will not be replaced (still al1::get is used instead of al2::get).

This is because of the "async" nature of the Gauge, you don't interact with the Gauge to modify its value but you register the Gauge once to observe the value and you modify the value directly. This is the intentional behavior but I can totally see that it can be tricky especially in the situation above where instrumentation code is "correct" and an "evil" MeterFilter makes the scenario "incorrect".

I think we should document this behavior (you cannot register a Gauge twice even if a MeterFilter causes this effectively). We can also add a warning message, there are two situations (with and without a filter) here (the two if blocks):

Meter m = preFilterIdToMeterMap.get(originalId);
if (m != null && !isStaleId(originalId)) {
return m;
}
Id mappedId = getMappedId(originalId);
m = meterMap.get(mappedId);
if (m != null) {
// If the mapping exists and the meter is marked stale, then this meter is no
// longer stale.
if (isStaleId(originalId)) {
unmarkStaleId(originalId);
}
}

@jonatan-ivanov jonatan-ivanov added bug A general bug and removed waiting-for-triage labels Oct 31, 2024
@jonatan-ivanov jonatan-ivanov added this to the 1.12.12 milestone Oct 31, 2024
jonatan-ivanov added a commit to jonatan-ivanov/micrometer that referenced this issue Oct 31, 2024
jonatan-ivanov added a commit to jonatan-ivanov/micrometer that referenced this issue Oct 31, 2024
@jonatan-ivanov
Copy link
Member

@keith-turner Could you please take a look at #5617

@keith-turner
Copy link
Author

keith-turner commented Nov 1, 2024

This is because of the "async" nature of the Gauge, you don't interact with the Gauge to modify its value but you register the Gauge once to observe the value and you modify the value directly. This is the intentional behavior but I can totally see that it can be tricky especially in the situation above where instrumentation code is "correct" and an "evil" MeterFilter makes the scenario "incorrect".

For this situation the following scenario is where an error or log message would be helpful.

  1. Developers A,B,and C write lots of metrics code with hundreds of meters
  2. Developers X,Y,and Z use the above code and write some meters filters to deal with some operations issues in the metrics system.

In step 2 above some data from gauges is silently ignored because of the meter filters. Developers X,Y,Z have no idea they are not seeing all of the data after working around the operational issue using meter filters.

@jonatan-ivanov jonatan-ivanov modified the milestones: 1.12.12, 1.12.13, 1.13.8 Nov 11, 2024
@jonatan-ivanov jonatan-ivanov added doc-update A documentation update and removed bug A general bug labels Nov 14, 2024
jonatan-ivanov added a commit to jonatan-ivanov/micrometer that referenced this issue Nov 16, 2024
@jonatan-ivanov
Copy link
Member

fyi: #5688

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-update A documentation update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants