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.

Sign up for GitHub

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

Jump to bottom

Micrometer performance - use Meter.MeterProvider #41295

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Micrometer performance - use Meter.MeterProvider #41295

merged 1 commit into from
Jun 25, 2024

Conversation

mcruzdev
Copy link
Contributor

Fixes #40597

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jun 18, 2024

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

@mcruzdev mcruzdev marked this pull request as draft June 18, 2024 17:47
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor

Still missing the enhancements on RestClientMetricsFilter and the PR is still on Draft.

@mcruzdev
Copy link
Contributor Author

Thank you for the feedback @brunobat, I will fix test and handle the RestClientMetricsFilter

@mcruzdev mcruzdev marked this pull request as ready for review June 20, 2024 14:27
@mcruzdev mcruzdev requested a review from ebullient June 20, 2024 14:28
@brunobat
Copy link
Contributor

can you please squash your commits, @mcruzdev ?

brunobat
brunobat reviewed Jun 20, 2024
View reviewed changes
.../src/main/java/io/quarkus/micrometer/runtime/binder/stork/StorkObservationCollectorBean.java

if (observation.failure() != null) {
if (observation.isServiceDiscoverySuccessful()) {
serviceSelectionFailures.increment();
this.serviceSelectionFailures.withTags(tags).increment();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were no tags before... But probably it's better to have them.

Copy link
Contributor Author

@mcruzdev mcruzdev Jun 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I missed the tags in the first commit. sorry 😢

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I say before I mean, before the PR... CC @aureamunoz

@mcruzdev mcruzdev force-pushed the feature/micrometer-perf branch 2 times, most recently from 1b32ce9 to 5e90da2 Compare June 20, 2024 14:37
@mcruzdev
Copy link
Contributor Author

Solving ...

@mcruzdev mcruzdev force-pushed the feature/micrometer-perf branch 4 times, most recently from 6ab200e to 0969aa7 Compare June 20, 2024 15:42
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jun 25, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 11643b9.

✅ 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 21

📦 integration-tests/reactive-messaging-kafka

✖ io.quarkus.it.kafka.KafkaConnectorTest.testFruits - History

  • Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.it.kafka.KafkaConnectorTest.testFruits(KafkaConnectorTest.java:63)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

@brunobat brunobat merged commit 9f9b454 into quarkusio:main Jun 25, 2024
28 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jun 25, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jun 25, 2024
@brunobat
Copy link
Contributor

Thanks @mcruzdev !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunobat brunobat brunobat approved these changes

@aureamunoz aureamunoz Awaiting requested review from aureamunoz

@ebullient ebullient Awaiting requested review from ebullient

Assignees
No one assigned
Labels
area/metrics kind/enhancement New feature or request triage/flaky-test
Projects
None yet
Milestone
3.13.0.CR1
Development

Successfully merging this pull request may close these issues.

Micrometer performance improvements - Stork and binder
3 participants
@mcruzdev @brunobat @ebullient