Skip to content

Conversation

@dehaansa
Copy link
Contributor

@dehaansa dehaansa commented Jan 25, 2022

Description: Update Kafka JMX metrics to update names, units, fix counters, and get more metrics.

Testing: Updated Integration test

import org.testcontainers.utility.MountableFile;

abstract class KafkaIntegrationTest extends AbstractIntegrationTest {
Startable createTopics =
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for moving this? Previously the containers were listed in order of execution which was a bit nicer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. I had moved it initially when I was trying to leverage it for another container but ended up removing that and didn't move it back. Will resolve.


@SafeVarargs
protected final void waitAndAssertMetrics(Consumer<Metric>... assertions) {
@SuppressWarnings("varargs")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think I see a varargs usage in this method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, fixed.

@rmfitzpatrick
Copy link
Contributor

rmfitzpatrick commented Jan 27, 2022

@dehaansa I'll be investigating the test mbean server issues shortly.

edit: may provide more context and earlier failures: #218

@dehaansa
Copy link
Contributor Author

@dehaansa I'll be investigating the test mbean server issues shortly.

edit: may provide more context and earlier failures: #218

Merged in the changes, looks like everything is passing, can I get a re-review @rmfitzpatrick ?

Copy link
Contributor

@rmfitzpatrick rmfitzpatrick left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements!

@rmfitzpatrick rmfitzpatrick merged commit eef8c17 into open-telemetry:main Jan 31, 2022
@dehaansa dehaansa deleted the kafka-jmx-updates branch January 31, 2022 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants