[
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:39 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|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CacheMeterBinder.java],_
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|https://github.com/ben-manes/caffeine/blob/b4cedbc411130b8e78c51effdd527756bdf1ff55/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java],
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|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
offers more detailed statistics than
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
and it allows name prefixes. However,
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
needs to have the meterRegistry already there {_}at the time the cache is
built{_}. There is no direct support to add a
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
to a Caffeine Cache after the cache is built. So if you use the
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
you might have to do something like call the
[MetricsConfiguration|https://github.com/apache/activemq-artemis/blob/c47713454caeece82df29a0a7fd4a2a39000576b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/MetricsConfiguration.java]
from the
*[SecurityStoreImpl|https://github.com/apache/activemq-artemis/blob/main/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java#L105-L108]*
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 maybe even extend the concrete class
[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}
I'm leaning toward just using
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
(and having tags to separate authn from authz cache metrics).
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|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CacheMeterBinder.java],_
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|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
offers more detailed statistics than
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
and it allows name prefixes. However,
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
needs to have the meterRegistry already there {_}at the time the cache is
built{_}. There is no direct support to add a
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
to a Caffeine Cache after the cache is built. So if you use the
[CaffeineStatsCounter|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineStatsCounter.java]
you might have to do something like call the
[MetricsConfiguration|https://github.com/apache/activemq-artemis/blob/c47713454caeece82df29a0a7fd4a2a39000576b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/MetricsConfiguration.java]
from the
*[SecurityStoreImpl|https://github.com/apache/activemq-artemis/blob/main/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java#L105-L108]*
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 maybe even extend the concrete class
[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}
I'm leaning toward just using
[CaffeineCacheMetrics|https://github.com/micrometer-metrics/micrometer/blob/37883fa6fb4a6d3f83d01f6b53101cc9f52b3f78/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.java]
(and having tags to separate authn from authz cache metrics).
> 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)
