Skip to content

Conversation

@carterkozak
Copy link
Contributor

Previously the default -1 values were used, resulting in
confusing dashboards that didn't make it obvious the data was
unavailable.

==COMMIT_MSG==
Memory gauges report null when data is unavailable
==COMMIT_MSG==

Previously the default `-1` values were used, resulting in
confusing dashboards that didn't make it obvious the data was
unavailable.
@changelog-app
Copy link

changelog-app bot commented Mar 17, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Memory gauges report null when data is unavailable

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from robert3005 March 17, 2022 21:12
@carterkozak carterkozak requested review from fawind and schlosna and removed request for robert3005 March 17, 2022 21:12
+ memoryBean.getNonHeapMemoryUsage().getMax());
metrics.totalInit(
(NonNegativeGauge) () -> memoryBean.getHeapMemoryUsage().getInit()
+ memoryBean.getNonHeapMemoryUsage().getInit());
Copy link
Contributor

Choose a reason for hiding this comment

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

are these computations problematic if one or both are not available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably worth checking both, in practice they’re all or nothing in my experience, but that could change.

carterkozak pushed a commit to palantir/gradle-baseline that referenced this pull request Mar 18, 2022
See the confusing failures with no source location on
palantir/tritium#1392 due to
assertions on Gauge values.
metrics.totalInit((Gauge<Long>) () -> {
long heap = memoryBean.getHeapMemoryUsage().getInit();
long nonHeap = memoryBean.getNonHeapMemoryUsage().getInit();
return (heap < 0 || nonHeap < 0) ? null : heap + nonHeap;
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the intent here to avoid producing any value if one don't have just one component (e.g. if we have heap init but do not have non-heap init)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have super strong feelings, but curious for thoughts on #1393

Looking at spec, it seems like getInit() & getMax() may return -1 when undefined, but used & committed should be non-negative though I guess we should handle similarly in case

@carterkozak
Copy link
Contributor Author

👍

@bulldozer-bot bulldozer-bot bot merged commit d49c032 into develop Mar 18, 2022
@bulldozer-bot bulldozer-bot bot deleted the ckozak/native_image_memory_values branch March 18, 2022 18:27
@svc-autorelease
Copy link
Collaborator

Released 0.44.0

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.

5 participants