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

Replace the id label with a name label for Netty allocator #45877

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

cescoffier
Copy link
Member

@cescoffier cescoffier commented Jan 27, 2025

The Netty allocator metrics exposed an id label using the allocator hashcode. However, this does not make a lot of sense when you need to aggregate metrics from multiple metrics.
Thus, this commit replaces the id with a name label with stable names for the different allocators.

Unfortunately, the initial code was in micrometer. This commit forks the 2 classes required to replace id with name and adds stable names to the different allocators.

The Netty allocator metrics exposed an `id` label using the allocator hashcode. However, this does not make a lot of sense when you need to aggregate metrics from multiple metrics.
Thus, this commit replaces the `id` with a `name` label with stable names for the different allocators.

Unfortunately, the initial code was in micrometer. This commit forks the 2 classes required to replace `id` with `name` and adds stable names to the different allocators.

Fix quarkusio#45847
Copy link

quarkus-bot bot commented Jan 27, 2025

/cc @brunobat (micrometer), @ebullient (micrometer)

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 27, 2025
Copy link

quarkus-bot bot commented Jan 27, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 2ee2894.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17 Windows

📦 extensions/micrometer-opentelemetry/deployment

io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_Uni - History

  • Stream has no elements - java.lang.IllegalArgumentException
java.lang.IllegalArgumentException: Stream has no elements
	at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lambda$lastReading$2(MetricDataFilter.java:213)
	at java.base/java.util.Optional.orElseThrow(Optional.java:403)
	at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReading(MetricDataFilter.java:213)
	at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReadingDataPoint(MetricDataFilter.java:231)
	at io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_Uni(MicrometerTimedInterceptorTest.java:174)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:513)

@geoand geoand merged commit 9dbcf90 into quarkusio:main Jan 27, 2025
29 checks passed
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jan 27, 2025
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Jan 27, 2025
@cescoffier cescoffier deleted the netty-allocator-hashcode-as-tags branch January 29, 2025 07:04
@gsmet gsmet modified the milestones: 3.19 - main, 3.18.2 Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Netty metric with dynamic tag cause time series leak
4 participants