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

add runtime metrics for buffer pool #6177

Merged
merged 4 commits into from
Jun 28, 2022

Conversation

cuichenli
Copy link
Contributor

fix #4876
Signed-off-by: Cuichen Li [email protected]

@cuichenli cuichenli requested a review from a team June 16, 2022 05:06
Signed-off-by: Cuichen Li <[email protected]>
Copy link
Contributor

@laurit laurit left a comment

Choose a reason for hiding this comment

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

@@ -38,6 +39,7 @@ public void afterAgent(Config config, AutoConfiguredOpenTelemetrySdk unused) {
if (config.getBoolean(
"otel.instrumentation.runtime-metrics.experimental-metrics.enabled", false)) {
GarbageCollector.registerObservers(GlobalOpenTelemetry.get());
BufferPools.registerObservers(GlobalOpenTelemetry.get());
Copy link
Member

Choose a reason for hiding this comment

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

Our process for adding these has been to discuss in the metrics JVM SIG, and codify in the runtime semantic conventions.

Are you able to attend the JVM metrics SIG?

If not, we should invite the participants to have a discussion here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, due to the time difference, i may not be able to attend the SIG meeting.

Copy link
Member

Choose a reason for hiding this comment

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

i've requested JVM metrics folks to have a look at this PR

(also @cuichenli in case you didn't notice, the JVM metrics meetings are at alternating times to accommodate different timezones)

.buildWithCallback(callback(bufferBeans, BufferPoolMXBean::getMemoryUsed));

meter
.upDownCounterBuilder("process.runtime.jvm.buffer.max")
Copy link
Member

Choose a reason for hiding this comment

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

Let's use limit per naming conventinos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


meter
.upDownCounterBuilder("process.runtime.jvm.buffer.count")
.setDescription("Total capacity of the buffers in this pool")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.setDescription("Total capacity of the buffers in this pool")
.setDescription("The number of buffers in the pool")

See getCount

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Signed-off-by: Cuichen Li <[email protected]>
Signed-off-by: Cuichen Li <[email protected]>
@laurit laurit merged commit 661af20 into open-telemetry:main Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add runtime metrics for buffer pools
4 participants