Skip registering Hazelcast meters when statistics are not enabled#7568
Open
jewoodev wants to merge 1 commit into
Open
Skip registering Hazelcast meters when statistics are not enabled#7568jewoodev wants to merge 1 commit into
jewoodev wants to merge 1 commit into
Conversation
Follow-up to micrometer-metricsgh-5402, micrometer-metricsgh-5409, and micrometer-metricsgh-7562 for HazelcastCacheMetrics. When a Hazelcast `IMap` is bound without statistics enabled, the registered meters silently report zero, which is hard to diagnose. This change detects the stats-disabled state via `MapConfig#isStatisticsEnabled()`, logs a warning explaining how to enable statistics, and skips registering every stats-dependent meter, including `cache.size` because it reads `LocalMapStats#getOwnedEntryCount()` and is meaningless once statistics are disabled. The check is added as a reflection chain on `HazelcastIMapAdapter` (`AbstractDistributedObject#getNodeEngine` -> `NodeEngine#getConfig` -> `Config#getMapConfig(name)` -> `MapConfig#isStatisticsEnabled`). The first two classes moved from `com.hazelcast.spi` (3.x) to `com.hazelcast.spi.impl` (4.x+), so the adapter tries both variants in the same `resolveOneOf` style already used for `IMap`, `LocalMapStats`, and `NearCacheStats`. Verified against Hazelcast 5.3.8, the pinned `optionalApi` version. If any class or method cannot be resolved or the chain throws at runtime, statistics are assumed enabled to avoid false-positive warnings and to keep existing mock-based callers unaffected. See micrometer-metricsgh-5066 Signed-off-by: jewoodev <jewoos15@naver.com>
Member
|
Thanks for the PR. We'll follow-up once we start merging things for 1.18 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to gh-5402, gh-5409, and gh-7562 for
HazelcastCacheMetrics. See gh-5066.When a Hazelcast
IMapis bound without statistics enabled, the registered meters silently report zero, which is hard to diagnose. This change mirrors the JCache pattern from gh-7562: detect the stats-disabled state viaMapConfig#isStatisticsEnabled(), log a warning explaining how to enable statistics, and skip registering the stats-dependent meters (cache.size,cache.gets,cache.puts,cache.entries,cache.entry.memory,cache.partition.gets, the near-cache gauges, and thecache.gets.latency/cache.puts.latency/cache.removals.latencytimers).cache.sizeis included because it readsLocalMapStats#getOwnedEntryCount(), which is meaningless once statistics are disabled.HazelcastCacheMetricsaccesses the cache through reflection viaHazelcastIMapAdapter, so the new check is added as a reflection chain on the same adapter:AbstractDistributedObject#getNodeEngine→NodeEngine#getConfig→Config#getMapConfig(name)→MapConfig#isStatisticsEnabled. The first two classes are internal types whose package changed between Hazelcast 3.x (com.hazelcast.spi) and 4.x+ (com.hazelcast.spi.impl), so the adapter tries both variants in the sameresolveOneOfstyle that the existingIMap/LocalMapStats/NearCacheStatsresolution already uses. The check was verified against Hazelcast 5.3.8, the pinnedoptionalApiversion. If any class or method in the chain cannot be resolved, or the chain throws at runtime, this assumes statistics are enabled to avoid a false-positive warning. This also keeps existing mock-based callers unaffected.Status of other cache binders
Closestrailer so that whichever of the two PRs merges last can carry it (or so a maintainer can close Log a warning when instrumenting a cache that is not recording stats #5066 manually once both are in).recordStats()was called. The upstream request (Provide API to see if a cache records statistics google/guava#7381) was closed in 2024 with no plans to add cache features.Testing
HazelcastCacheMetricsTest#doNotReportMetricsWhenStatisticsAreDisabledregisters aMapConfigwith statistics disabled at instance startup, binds metrics to thatIMap, and asserts both the warning is emitted and thatcache.size,cache.gets,cache.puts,cache.entries,cache.entry.memory,cache.partition.gets, and the three latency timers are not registered. This is also what exercises the reflection chain on Hazelcast 5.3.8 end-to-end — the existing tests use default (enabled) statistics, where the chain returningtrueis indistinguishable from the fail-safe fallback.IMapwith default (enabled) statistics, so meters register as before../gradlew :micrometer-core:checkpasses locally.