[
https://issues.apache.org/jira/browse/ARTEMIS-4306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17849099#comment-17849099
]
Mike Artz edited comment on ARTEMIS-4306 at 5/23/24 8:24 PM:
-------------------------------------------------------------
[~jbertram] - I made this [PR for micrometer to allow prefixing in the
CacheMeterBinder|https://github.com/micrometer-metrics/micrometer/pull/4048],
so that we could for example add the *"artemis.authentication."*
prefix but the PR _kind of_ got stuck, and the PR got closed. Then I had a kid
and had a new job.
Coming back to this now, _if we use the CacheMeterBinder,_ would it be ok to
not have *"artemis.authentication."* prefixes, and adding *authentication* and
*authorization* as Tags? i.e. {{{}Tag("cacheName", "authentication"){}}}and
{{{}Tag("{}}}{{{}cacheName{}}}{{{}", "authorization"){}}} It seems this might
be more of a standard too for micrometer users possibly.
However, now that we are using the CaffeineCache, I see there are two concrete
classes -
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
and
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java].
Both of these look ok at first (more or less), but I am stuck at this
decision. *CaffeineStatsCounter* offers more detailed statistics than
*CaffeineCacheMetrics* and it allows name prefixes. However,
*CaffeineStatsCounter* needs to have the meterRegistry already there {_}at the
time the cache is built{_}. There is no direct support to add a
*CaffeineStatsCounter* to a Caffeine Cache after the cache is built. So if you
use the *CaffeineStatsCounter* you might have to do something like call the
*MetricsConfiguration* from the *SecurityStoreImpl* like
{code:java}
MeterRegistry registry = metricsConfiguration.getPlugin().getRegistry();
authenticationCache = Caffeine.newBuilder()
.maximumSize(authenticationCacheSize)
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
.recordStats(() -> new CaffeineStatsCounter(registry, "authentication"))
.build();{code}
And that doesnt make any sense because *MetricsConfiguration* happens after the
*SecurityStoreImpl.* So this doesnt seem like the best option.
*Other Options*
* Use
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
could initialize the authn cache (and authz cache) from the MetricsManager
class (Couples the SecurityStoreImpl to the MetricsManager but maybe 2nd best
option)
* Use
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
(Seems like best option)
* Create another concrete class (or extend
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java])
that does as much as
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
(Seems overkill)
* Create some sort of decorator that would intercept the cache operations and
delegate to an underlying cache (Seems overkill)
{code:java}
public class StatsCounterDecorator<K, V> implements Cache<K, V> {
private final Cache<K, V> delegate;
private final StatsCounter statsCounter;
...
@Override
public void put(K key, V value) {
delegate.put(key, value);
}
public static void main(String[] args) {
//get already initialized cache
server
.getSecurityStore()
.getAuthenticationMetrics()
StatsCounter statsCounter = new ConcurrentStatsCounter();
Cache<String, String> cacheWithStats = new
StatsCounterDecorator<>(originalCache, statsCounter);
}{code}
was (Author: JIRAUSER301522):
[~jbertram] - I made this [PR for micrometer to allow prefixing in the
CacheMeterBinder|https://github.com/micrometer-metrics/micrometer/pull/4048],
so that we could for example add the *"artemis.authentication."*
prefix but the PR _kind of_ got stuck, and the PR got closed. Then I had a kid
and had a new job.
Coming back to this now, _if we use the CacheMeterBinder,_ would it be ok to
not have *"artemis.authentication."* prefixes, and adding *authentication* and
*authorization* as Tags? i.e. {{{}Tag("cacheName", "authentication"){}}}and
{{{}Tag("{}}}{{{}cacheName{}}}{{{}", "authorization"){}}} It seems this might
be more of a standard too for micrometer users possibly.
However, now that we are using the CaffeineCache, I see there are two concrete
classes -
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
and
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java].
Both of these look ok at first (more or less), but I am stuck at this
decision. *CaffeineStatsCounter* offers more detailed statistics than
*CaffeineCacheMetrics* and it allows name prefixes. However,
*CaffeineStatsCounter* needs to have the meterRegistry already there {_}at the
time the cache is built{_}. There is no direct support to add a
*CaffeineStatsCounter* to a Caffeine Cache after the cache is built. So if you
use the *CaffeineStatsCounter* you might have to do something like call the
*MetricsConfiguration* from the *SecurityStoreImpl* like
{code:java}
MeterRegistry registry = metricsConfiguration.getPlugin().getRegistry();
authenticationCache = Caffeine.newBuilder()
.maximumSize(authenticationCacheSize)
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
.recordStats(() -> new CaffeineStatsCounter(registry, "authentication"))
.build();{code}
And that doesnt make any sense because *MetricsConfiguration* happens after the
*SecurityStoreImpl.* So this doesnt seem like the best option.
*Other Options*
* Use
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
could initialize the authn cache (and authz cache) from the MetricsManager
class (Couples the SecurityStoreImpl to the MetricsManager but maybe 2nd best
option)
* Use
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
(Seems like best option)
* Create another concrete class (or extend
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java])
that does as much as
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
(Seems overkill)
* Create some sort of decorator that would intercept the cache operations and
delegate to an underlying cache (Seems overkill)
{code:java}
public class StatsCounterDecorator<K, V> implements Cache<K, V> {
private final Cache<K, V> delegate;
private final StatsCounter statsCounter;
...
@Override
public void put(K key, V value) {
delegate.put(key, value);
}
public static void main(String[] args) {
//get already initialized cache
server
.getSecurityStore()
.getAuthenticationMetrics()
StatsCounter statsCounter = new ConcurrentStatsCounter();
Cache<String, String> cacheWithStats = new
StatsCounterDecorator<>(originalCache, statsCounter);
}{code}
> Add authn/z metrics
> -------------------
>
> Key: ARTEMIS-4306
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4306
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Priority: Major
>
> It would be useful to have metrics for authn/z successes and failures as well
> as for metrics related to the corresponding caches.
> See this discussion on the users mailing list for more details:
> https://lists.apache.org/thread/g6ygyo4kb3xhygq8hpw7vsl3l2g5qt92
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
