Skip to content

Commit

Permalink
Add failing test to capture gh-4357
Browse files Browse the repository at this point in the history
  • Loading branch information
lenin-jaganathan committed Dec 9, 2023
1 parent f219756 commit 6d75b0a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,20 @@ void whenCloseDuringScheduledPublish_thenPreviousStepAndCurrentPartialStepArePub
assertThat(registry.publishedFunctionTimerTotals.pop()).isEqualTo(24);
}

@Test
@Issue("#4357")
void publishOnceWhenClosedWithinFirstStep() {
// Set the initial clock time to a valid time.
MockClock mockClock = new MockClock();
mockClock.add(otlpConfig().step().multipliedBy(5));

TestOtlpMeterRegistry stepMeterRegistry = new TestOtlpMeterRegistry(otlpConfig(), mockClock);

assertThat(stepMeterRegistry.publishCount.get()).isZero();
stepMeterRegistry.close();
assertThat(stepMeterRegistry.publishCount.get()).isEqualTo(1);
}

private void assertEmptyHistogramSnapshot(HistogramSnapshot snapshot) {
assertThat(snapshot.count()).isZero();
assertThat(snapshot.total()).isZero();
Expand Down Expand Up @@ -771,6 +785,8 @@ private void assertHistogramContains(HistogramSnapshot snapshot, double total, d

private class TestOtlpMeterRegistry extends OtlpMeterRegistry {

private final AtomicInteger publishCount = new AtomicInteger();

Deque<Double> publishedCounterCounts = new ArrayDeque<>();

Deque<Long> publishedTimerCounts = new ArrayDeque<>();
Expand All @@ -795,18 +811,24 @@ private class TestOtlpMeterRegistry extends OtlpMeterRegistry {

Deque<Double> publishedFunctionTimerTotals = new ArrayDeque<>();

private long lastScheduledPublishStartTime = 0L;
private long lastScheduledPublishStartTime;

AtomicBoolean isPublishing = new AtomicBoolean(false);

CompletableFuture<Void> scheduledPublishingFuture = CompletableFuture.completedFuture(null);

TestOtlpMeterRegistry() {
super(otlpConfig(), OtlpDeltaMeterRegistryTest.this.clock);
this(otlpConfig(), OtlpDeltaMeterRegistryTest.this.clock);
}

TestOtlpMeterRegistry(OtlpConfig otlpConfig, Clock clock) {
super(otlpConfig, clock);
this.lastScheduledPublishStartTime = super.getLastScheduledPublishStartTime();
}

@Override
protected void publish() {
publishCount.incrementAndGet();
forEachMeter(meter -> meter.match(null, this::publishCounter, this::publishTimer, this::publishSummary,
null, null, this::publishFunctionCounter, this::publishFunctionTimer, null));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@
*/
class StepMeterRegistryTest {

private final AtomicInteger publishes = new AtomicInteger();

private final MockClock clock = new MockClock();

private final StepRegistryConfig config = new StepRegistryConfig() {
Expand Down Expand Up @@ -98,9 +96,9 @@ void serviceLevelObjectivesOnlyNoPercentileHistogram() {
@Issue("#484")
@Test
void publishOneLastTimeOnClose() {
assertThat(publishes.get()).isEqualTo(0);
assertThat(registry.publishCount.get()).isZero();
registry.close();
assertThat(publishes.get()).isEqualTo(1);
assertThat(registry.publishCount.get()).isEqualTo(1);
}

@Issue("#1993")
Expand Down Expand Up @@ -425,7 +423,7 @@ void scheduledRollOver() {
}

@Test
@Issue("3914")
@Issue("#3914")
void publishShouldNotHappenWhenRegistryIsDisabled() {
StepRegistryConfig disabledStepRegistryConfig = new StepRegistryConfig() {
@Override
Expand All @@ -449,27 +447,27 @@ public String get(String key) {
Counter.builder("publish_disabled_counter").register(disabledStepMeterRegistry).increment();

clock.add(config.step());
assertThat(publishes.get()).isZero();
assertThat(disabledStepMeterRegistry.publishCount.get()).isZero();
disabledStepMeterRegistry.close();
assertThat(publishes.get()).isZero();
assertThat(disabledStepMeterRegistry.publishCount.get()).isZero();
}

@Test
@Issue("3914")
@Issue("#3914")
void publishShouldNotHappenWhenRegistryIsClosed() {
Counter.builder("my.counter").register(registry).increment();

clock.add(config.step());
assertThat(publishes.get()).isZero();
assertThat(registry.publishCount.get()).isZero();
registry.close();
assertThat(publishes.get()).isEqualTo(2);
assertThat(registry.publishCount.get()).isEqualTo(2);
assertThat(registry.publishedCounterCounts).hasSize(2);
assertThat(registry.publishedCounterCounts.getFirst()).isOne();
assertThat(registry.publishedCounterCounts.getLast()).isZero();

clock.add(config.step());
registry.close();
assertThat(publishes.get()).isEqualTo(2);
assertThat(registry.publishCount.get()).isEqualTo(2);
}

@Test
Expand Down Expand Up @@ -557,8 +555,23 @@ void whenCloseDuringScheduledPublish_thenPreviousStepAndCurrentPartialStepArePub
assertThat(registry.publishedFunctionTimerTotals.pop()).isEqualTo(24);
}

@Test
@Issue("#4357")
void publishOnceWhenClosedWithinFirstStep() {
// Set the initial clock time to a valid time.
MockClock mockClock = new MockClock();
mockClock.add(config.step().multipliedBy(5));

MyStepMeterRegistry stepMeterRegistry = new MyStepMeterRegistry(config, mockClock);
assertThat(stepMeterRegistry.publishCount.get()).isZero();
stepMeterRegistry.close();
assertThat(stepMeterRegistry.publishCount.get()).isEqualTo(1);
}

private class MyStepMeterRegistry extends StepMeterRegistry {

private final AtomicInteger publishCount = new AtomicInteger();

Deque<Double> publishedCounterCounts = new ArrayDeque<>();

Deque<Long> publishedTimerCounts = new ArrayDeque<>();
Expand All @@ -575,7 +588,7 @@ private class MyStepMeterRegistry extends StepMeterRegistry {

Deque<Double> publishedFunctionTimerTotals = new ArrayDeque<>();

private long lastScheduledPublishStartTime = 0L;
private long lastScheduledPublishStartTime;

@Nullable
Runnable prePublishAction;
Expand All @@ -590,6 +603,7 @@ private class MyStepMeterRegistry extends StepMeterRegistry {

MyStepMeterRegistry(StepRegistryConfig config, Clock clock) {
super(config, clock);
this.lastScheduledPublishStartTime = super.getLastScheduledPublishStartTime();
}

void setPrePublishAction(Runnable prePublishAction) {
Expand All @@ -601,7 +615,7 @@ protected void publish() {
if (prePublishAction != null) {
prePublishAction.run();
}
publishes.incrementAndGet();
publishCount.incrementAndGet();
getMeters().stream()
.map(meter -> meter.match(g -> null, this::publishCounter, this::publishTimer, this::publishSummary,
null, tg -> null, this::publishFunctionCounter, this::publishFunctionTimer, m -> null))
Expand Down

0 comments on commit 6d75b0a

Please sign in to comment.