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

Azure Registry publishing metrics with count equal to zero #2463

Closed
uvalashchuk opened this issue Feb 22, 2021 · 5 comments · Fixed by #2472
Closed

Azure Registry publishing metrics with count equal to zero #2463

uvalashchuk opened this issue Feb 22, 2021 · 5 comments · Fixed by #2472
Labels
bug A general bug registry: azure monitor An Azure Monitor Registry related issue
Milestone

Comments

@uvalashchuk
Copy link

We're using micrometer to collect metrics into Azure application insights and faced an issue when data from timer being published even when timer was not called in this period. This results into invalid aggregation, because Azure app insights replaces valueCount = 0 to valueCount = 1.

For example, we're using 1 minute collecting interval and a method which has Thread.sleep(300) in it's body. This method produces metrics described below.

image

For entries which have value = 0 there where no method calls, but it was pushed into app insights. This results in wrong aggregation when we're building dashboards using default metrics visualizer from Azure.

image
(Method executes 300ms, but on time chart we're observing average time of 150ms)

@uvalashchuk
Copy link
Author

Also, I've noticed that there is a comment on com.microsoft.applicationinsights.telemetry.MetricTelemetry#setCount that count should be greater or equal to 1. But io.micrometer.azuremonitor.AzureMonitorMeterRegistry#trackTimer (and other methods) sets count from io.micrometer.core.instrument.Timer#count without any zero-checks despite Timer#count can return 0.

     /**
     * Sets the number of samples for this metric.
     * @param count Number of samples greater than or equal to 1
     */
    public void setCount(Integer count)

@vagpap
Copy link

vagpap commented Feb 23, 2021

I am also facing the issue.

In a try to overcome this, I "recreated" AzureMonitorMeterRegistry class but replaced the publish method as by adding a filter in the stream before forEach as follow:

protected void publish() {
	logger.info("Publishing information");
	for (Meter meter : getMeters()) {
		meter.match(
				this::trackGauge,
				this::trackCounter,
				this::trackTimer,
				this::trackDistributionSummary,
				this::trackLongTaskTimer,
				this::trackTimeGauge,
				this::trackFunctionCounter,
				this::trackFunctionTimer,
				this::trackMeter
		)
		.filter(m -> m.getCount().intValue() != 0) // Do not track the meter in case count == 0
		.forEach(telemetry -> {
			try {
				client.track(telemetry);
			} catch (Throwable e) {
				logger.warn("failed to track metric {} in azure monitor", meter.getId());
				TraceTelemetry traceTelemetry = new TraceTelemetry("failed to track metric " + meter.getId());
				traceTelemetry.getContext().getOperation().setSyntheticSource(SDK_TELEMETRY_SYNTHETIC_SOURCE_NAME);
				traceTelemetry.setSeverityLevel(SeverityLevel.Warning);
				client.trackTrace(traceTelemetry);
				client.flush();
			}
		});
	}
}

Most probably not the best solution, but it works, at least for Timer metrics, which we use

@uvalashchuk
Copy link
Author

We found other way to solve this which may be useful.

Microsoft's library provides interface com.microsoft.applicationinsights.extensibility.TelemetryProcessor which allows to filter out telemetries. As we're using applicationinsights-spring-boot-starter-2.6.2, it autowires TelemetryProcessor into telemetry client. So this filtering can be done just by registering bean

Kotlin code:

@Component
class ZeroCountMetricsFilter : TelemetryProcessor {

    override fun process(telemetry: Telemetry): Boolean {
        if (telemetry !is MetricTelemetry) {
            return true
        }

        return telemetry.count != 0
    }
}

izeye added a commit to izeye/micrometer that referenced this issue Feb 23, 2021
This commit also applies it to FunctionTimer and DistributionSummary.

Closes micrometer-metricsgh-2463
@izeye
Copy link
Contributor

izeye commented Feb 23, 2021

@uvalashchuk @vagpap Thanks for sharing! I created #2472 to try to resolve this.

@jonatan-ivanov
Copy link
Member

I understand that Azure wants count to be greater than zero but I might create an issue for them because this is super weird:

Azure app insights replaces valueCount = 0 to valueCount = 1.

@jonatan-ivanov jonatan-ivanov added the bug A general bug label Feb 27, 2021
@jonatan-ivanov jonatan-ivanov added this to the 1.3.18 milestone Feb 27, 2021
@shakuzen shakuzen added the registry: azure monitor An Azure Monitor Registry related issue label Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug registry: azure monitor An Azure Monitor Registry related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants