Add support for dw-metrics 4.x by kingster · Pull Request #1228 · prometheus/client_java

kingster · Dec 11, 2024

Adding support for com.codahale.metrics.MetricRegistry(Dropwizard Metrics 4.x)

Completes #867

kingster · Dec 11, 2024

Hi @fstab Please do review this PR, this solves for Dropwizard 4.x metrics support.

zeitlinger

Thanks for the contribution 😄

zeitlinger · Dec 12, 2024

prometheus-metrics-instrumentation-dropwizard/pom.xml

test dependencies are added in the parent already:

client_java/pom.xml

Lines 103 to 143 in 9a0848d

<dependency>

<groupId>org.junit.jupiter</groupId>

<artifactId>junit-jupiter-engine</artifactId>

<version>${junit-jupiter.version}</version>

<scope>test</scope>

</dependency>

<dependency>

<groupId>org.junit.jupiter</groupId>

<artifactId>junit-jupiter-params</artifactId>

<version>${junit-jupiter.version}</version>

<scope>test</scope>

</dependency>

<dependency>

<groupId>org.mockito</groupId>

<artifactId>mockito-core</artifactId>

<version>5.14.2</version>

<scope>test</scope>

</dependency>

<dependency>

<groupId>org.assertj</groupId>

<artifactId>assertj-core</artifactId>

<version>3.26.3</version>

<scope>test</scope>

</dependency>

<dependency>

<groupId>com.google.guava</groupId>

<artifactId>guava</artifactId>

<scope>test</scope>

</dependency>

<dependency>

<groupId>org.slf4j</groupId>

<artifactId>slf4j-simple</artifactId>

<version>2.0.16</version>

<scope>test</scope>

</dependency>

<dependency>

<groupId>org.junit-pioneer</groupId>

<artifactId>junit-pioneer</artifactId>

<version>2.3.0</version>

<scope>test</scope>

</dependency>

Sure, removed these.

zeitlinger · Dec 12, 2024

prometheus-metrics-instrumentation-dropwizard/pom.xml

should only depend on prometheus-metrics-exposition-textformats

Sure, changed this.

zeitlinger · Dec 12, 2024

...rd/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java

please use assertj for all assertions

Thanks for pointing this out. Fixed this

zeitlinger · Dec 12, 2024

...rd/src/test/java/io/prometheus/metrics/instrumentation/dropwizard/DropwizardExportsTest.java

did the assertion not work?

It works, just that its not a string comparison, hence this comment is to explain why its not asserted like in other cases.

can you test it like here:

client_java/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/HistogramTest.java

Line 950 in e486e4d

assertThat(out).hasToString(expectedTextFormat);

?

I think doing it that way will lead to a flaky test-suite, because the problem is in the dw metrics4 histogram. This is something that had happened earlier and that is specially why this approach was taken.

fca49ff

I took reference of this method from the dw5 implementation.

Updated the pr to use

assertThat(textFormat) .satisfiesAnyOf( text -> assertThat(text).isEqualTo(expected1), text -> assertThat(text).isEqualTo(expected2));

So that it matches the exact output, and is a much simpler test.

Signed-off-by: Kinshuk Bairagi <hi@kinsh.uk>

kingster · Dec 12, 2024

@zeitlinger Have made the changes as requested, could you please have a look again?

Signed-off-by: Kinshuk Bairagi <hi@kinsh.uk>

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>

kingster · Dec 13, 2024

Thank you @zeitlinger

Are snapshot versions published at any repository? If not when is the next planned version release?

zeitlinger · Dec 13, 2024

Thank you @zeitlinger

Are snapshot versions published at any repository? If not when is the next planned version release?

1.3.5 has just been released 😄

kingster force-pushed the prometheus-metrics-instrumentation-dropwizard-4x branch 4 times, most recently from 2e0b416 to fa85d06 Compare December 11, 2024 19:21

kingster marked this pull request as ready for review December 11, 2024 19:30

zeitlinger requested changes Dec 12, 2024

View reviewed changes

kingster force-pushed the prometheus-metrics-instrumentation-dropwizard-4x branch 2 times, most recently from a07277c to 13c320c Compare December 12, 2024 13:43

Add support for dw-metrics 4.x

529676e

Signed-off-by: Kinshuk Bairagi <hi@kinsh.uk>

kingster force-pushed the prometheus-metrics-instrumentation-dropwizard-4x branch from 13c320c to 529676e Compare December 12, 2024 13:45

kingster and others added 2 commits December 13, 2024 09:59

Update DropwizardExportsTest.java

9fda943

Signed-off-by: Kinshuk Bairagi <hi@kinsh.uk>

cleanup

657788c

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>

zeitlinger approved these changes Dec 13, 2024

View reviewed changes

zeitlinger merged commit 5359ba3 into prometheus:main Dec 13, 2024

kingster mentioned this pull request Dec 13, 2024

Switch to dw-metrics stable 4.x flipkart-incubator/grpc-jexpress#118

Merged

kingster deleted the prometheus-metrics-instrumentation-dropwizard-4x branch December 13, 2024 11:55

	<dependency>
	<groupId>org.junit.jupiter</groupId>
	<artifactId>junit-jupiter-engine</artifactId>
	<version>${junit-jupiter.version}</version>
	<scope>test</scope>
	</dependency>
	<dependency>
	<groupId>org.junit.jupiter</groupId>
	<artifactId>junit-jupiter-params</artifactId>
	<version>${junit-jupiter.version}</version>
	<scope>test</scope>
	</dependency>
	<dependency>
	<groupId>org.mockito</groupId>
	<artifactId>mockito-core</artifactId>
	<version>5.14.2</version>
	<scope>test</scope>
	</dependency>
	<dependency>
	<groupId>org.assertj</groupId>
	<artifactId>assertj-core</artifactId>
	<version>3.26.3</version>
	<scope>test</scope>
	</dependency>
	<dependency>
	<groupId>com.google.guava</groupId>
	<artifactId>guava</artifactId>
	<scope>test</scope>
	</dependency>
	<dependency>
	<groupId>org.slf4j</groupId>
	<artifactId>slf4j-simple</artifactId>
	<version>2.0.16</version>
	<scope>test</scope>
	</dependency>
	<dependency>
	<groupId>org.junit-pioneer</groupId>
	<artifactId>junit-pioneer</artifactId>
	<version>2.3.0</version>
	<scope>test</scope>
	</dependency>

Search code, repositories, users, issues, pull requests...

Conversation

kingster commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kingster commented Dec 11, 2024

Uh oh!

zeitlinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kingster commented Dec 12, 2024

Uh oh!

kingster commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeitlinger commented Dec 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

kingster commented Dec 11, 2024 •

edited

Loading

kingster commented Dec 13, 2024 •

edited

Loading