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
Events notification system for Nessie - Quarkus #6870
Conversation
@Produces | ||
@Singleton | ||
public MeterFilter configureGlobalTags() { | ||
return MeterFilter.commonTags(Tags.of(APPLICATION_TAG_NAME, APPLICATION_TAG_VALUE)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a global bean and as such, interferes with other usages of MeterRegistry
throughout the application. Which is why the returned MeterFilter
contains exactly the same tags used in MetricsVersionStore
– this way we don't alter its behavior.
We might want to improve this later on and have a global strategy for metrics tags in Nessie.
} | ||
|
||
@Test | ||
void testCommitWithTracing() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3cbb7ad
to
4891186
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a full review, but some higher level first-round comments.
events/service/src/main/java/org/projectnessie/events/service/util/SubscriberUtil.java
Outdated
Show resolved
Hide resolved
events/service/src/main/java/org/projectnessie/events/service/EventSubscribers.java
Outdated
Show resolved
Hide resolved
events/spi/src/main/java/org/projectnessie/events/spi/EventSubscription.java
Show resolved
Hide resolved
...arkus/src/main/java/org/projectnessie/events/quarkus/config/VersionStoreConfigConstants.java
Outdated
Show resolved
Hide resolved
aafeed5
to
db893a5
Compare
Heads up: had to rebase because of conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM - some minor comments
.../src/main/java/org/projectnessie/events/quarkus/collector/QuarkusResultCollectorFactory.java
Outdated
Show resolved
Hide resolved
...s/quarkus/src/main/java/org/projectnessie/events/quarkus/delivery/StandardEventDelivery.java
Show resolved
Hide resolved
events/quarkus/src/main/java/org/projectnessie/events/quarkus/config/QuarkusEventConfig.java
Show resolved
Hide resolved
.../quarkus/src/main/java/org/projectnessie/events/quarkus/delivery/RetriableEventDelivery.java
Outdated
Show resolved
Hide resolved
...quarkus/src/main/java/org/projectnessie/events/quarkus/delivery/DelegatingEventDelivery.java
Outdated
Show resolved
Hide resolved
...s/quarkus/src/main/java/org/projectnessie/events/quarkus/delivery/StandardEventDelivery.java
Outdated
Show resolved
Hide resolved
...s/quarkus/src/main/java/org/projectnessie/events/quarkus/delivery/StandardEventDelivery.java
Show resolved
Hide resolved
This commit introduces a new module, nessie-events-quarkus. To improve isolation and facilitate testing, this new module is completely independent of other Quarkus modules; it contains: 1. Quarkus-specific implementations of nessie-events-service classes; 2. Asynchronous delivery based on Vert.x, both non-blocking and blocking; 3. Delivery with optional logging, tracing and metrics. Changes outside this module are minimal: 1. A few changes were introduced in nessie-events-api, nessie-events-spi and nessie-events-service. 2. nessie-quarkus-common: 1. ConfigurableVersionStoreFactory changed to detect when events are enabled; 2. New RepositoryIdProvider. 3. nessie-quarkus (server): 1. Compile-time dependency on nessie-events-quarkus; 2. Smoke tests.
Let's merge manually to have a "nicer" commit message. |
I changed the commit message when I enabled auto-merge :-) – but OK, let's wait. |
This commit introduces a new module,
nessie-events-quarkus
. To improve isolation and facilitate testing, this new module is completely independent of other Quarkus modules; it contains:nessie-events-service
classes;nessie.version.store.XYZ.enable
options.Changes outside this module are minimal:
nessie-events-api
,nessie-events-spi
andnessie-events-service
.nessie-quarkus-common
:VersionStoreConfig
has a newisEventsEnabled()
method mapped to thenessie.version.store.events.enable
property, true by default;ConfigurableVersionStoreFactory
changed to detect when events are enabled;RepositoryIdProvider
: creates a bean holding the repository id.nessie-quarkus
(server):nessie-events-quarkus
;