Are counters thread safe in Micrometer
Asked Answered
P

3

6

I am trying to publish number of events my App is processing. This is the code I have at the receiving end:

public void process(List<String> batch) {
    logger.info ("Processing batch of size " + batch.size())
    metrics.incrementCounter(MetricsType.CONSUMER_TOTAL_PROCESSED, batch.size)
}

The class Metrics is:

public class Metrics {
    private static final Map<MetricsType, Counter> COUNTER_MAP = new ConcurrentHashMap<>();

    public Metrics(
        @Autowired MeterRegistry meterRegistry
    ) {
        COUNTER_MAP.put(
            MetricsType.CONSUMER_TOTAL_PROCESSED,
            Counter.builder("CONSUMER_TOTAL_PROCESSED").register(meterRegistry)
        );

        COUNTER_MAP.put(
            MetricsType.CONSUMER_DUPLICATE_PROCESSED,
            Counter.builder("CONSUMER_DUPLICATE_PROCESSED").register(meterRegistry)
        );
    }

    public void increment(MetricsType metricsType, int size) {
        COUNTER_MAP.get(metricsType).increment(size);
    }
}

The enum MetricsType contains all type of counters.

The process method is be invoked by 16 threads at any time. The issue I am facing is the logger which prints the count and the total count reported in grafana are way off.

Do I have to synchonize everytime I am incrementing the counter?


Edit - What I mean by the counts are off is, if there are two logs with size 200, then grafana should report total counter 400. I am validating this by taking a time range of 2 hours, I extract all the sizes from logs and add them.


If you stumble upon this and see a difference between two sources please check what is your maximum number of data points in Grafana, that was the actual issue why I thought counters may not be threadsafe.

Propylene answered 12/3, 2020 at 9:59 Comment(0)
M
8

The counters are thread safe, internally they use some atomic data structure for counting. Given your code I would suspect that what the logger prints is quite different from what you see in grafana because the logger prints the size of the current batch while the counter prints the sum of all batch sizes.

Motoneuron answered 12/3, 2020 at 11:12 Comment(6)
So what does Grafana show? Less or more? How often do you scrape data from your app? Could it be a timing issue?Nearly
Can you please add a reference to your claim about thread-safety? And even if you copy-paste actual source code, this would be an implementation detail and anyone programming against this version of the source code how it is today is making a bold assumption. We need a reference to actual docs, preferably JavaDoc.Elliotelliott
Well I actually just checked the implementation. You are right that the Javadoc does not give such a guarantee. Then again I would argue that a metrics library which is not thread-safe would not be very useful.Nearly
I understand your point, but I would argue any library without documentation is not very useful. Usually if someone can't document and then write clean code then he probably started writing spaghetti code and discovers at some point in time in the future that the code is such a mess it can't even be documented. So not having proper docs is usually a tell-tale sign of what kind of "quality" you pull into your own project. Thread-safety is SUPER important - especially for a metrics library. That this is not documented anywhere is very sad. They need to take a look at their priorities.Elliotelliott
Actually I just came back to this problem again. Because now I am working with dynamic meters that needs to be removed. So I need to know what memory visibility guarantees do I have, I clearly don't want to remove a meter before it has been scraped, and so on. Again, sad.Elliotelliott
Well I understand your concerns. It's not like you have a lot of options here though, so sometimes you will just need to make do with what you have. Then again it's an open source project so if you have the resources and inclination you could send them some pull request and improve the package.Nearly
S
2

There are two levels of concurrency

  1. concurrent access to the meter registry that holds the counter reference. This is thread safe
    private final Map<Meter.Id, Meter> meterMap = new ConcurrentHashMap();
  1. concurrent access to the counter itself. Since in micrometer, the backing counter is from the registered framework (e.g. say netflix atlas). One would "safely" assume counter implementation would be thread safe because it would otherwise be useless. But for sake of argument, we can see that one such exampe, the SimpleMeterRegistry implementation of a cumulative counter provided by micrometer.io is thread-safe, as it uses JDK DoubleAdder as its backing implementation
public class CumulativeCounter extends AbstractMeter implements Counter {
    private final DoubleAdder value = new DoubleAdder();

And again, the answer depends on the registry, even though we can safely assume all published registry guarantees counter-level thread-safely. Or, to get a definite answer, we need to a definite question

Are counters from Netflix Atlas registry thread safe in Micrometer
Sheep answered 28/1, 2023 at 19:28 Comment(1)
Not pointless to have a non thread safe. You could have a threading model where a single thread and an event loop is used, and thus synchronization is not needed and yet consumes most of the performance gains on this model.Engenia
F
0

Counters are monotonically incrementing. So if a previous batch has occurred, then a new batch would add to the previous total.

What back end are you using? If you are using Prometheus you might wrap your query in an increase function so you can visualize how much the counter is increasing.

(To answer the original question though: Yes, counters are thread safe)

Ferrocyanide answered 13/3, 2020 at 15:21 Comment(1)
Your actual answer is the last sentence, put in parenthesis. Without a reference. I was THIS close to downvoting lol.Elliotelliott

© 2022 - 2024 — McMap. All rights reserved.