Skip to content

Commit 4237a26

Browse files
the-clamthe-clam
authored
added new constructor and tests (#7514)
1 parent 590f112 commit 4237a26

File tree

2 files changed

+154
-10
lines changed

2 files changed

+154
-10
lines changed

‎sdk/common/src/main/java/io/opentelemetry/sdk/internal/ThrottlingLogger.java

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static java.util.concurrent.TimeUnit.MINUTES;
99

1010
import io.opentelemetry.sdk.common.Clock;
11+
import java.util.Locale;
1112
import java.util.concurrent.TimeUnit;
1213
import java.util.concurrent.atomic.AtomicBoolean;
1314
import java.util.logging.Level;
@@ -21,28 +22,52 @@
2122
* at any time.
2223
*/
2324
public class ThrottlingLogger {
24-
private static final double RATE_LIMIT = 5;
25-
private static final double THROTTLED_RATE_LIMIT = 1;
26-
private static final TimeUnit rateTimeUnit = MINUTES;
25+
private static final double DEFAULT_RATE_LIMIT = 5;
26+
private static final double DEFAULT_THROTTLED_RATE_LIMIT = 1;
27+
private static final TimeUnit DEFAULT_RATE_TIME_UNIT = MINUTES;
2728

2829
private final Logger delegate;
2930
private final AtomicBoolean throttled = new AtomicBoolean(false);
3031
private final RateLimiter fastRateLimiter;
3132
private final RateLimiter throttledRateLimiter;
3233

34+
private final double rateLimit;
35+
private final double throttledRateLimit;
36+
private final TimeUnit rateTimeUnit;
37+
3338
/** Create a new logger which will enforce a max number of messages per minute. */
3439
public ThrottlingLogger(Logger delegate) {
3540
this(delegate, Clock.getDefault());
3641
}
3742

43+
/** Alternate way to create logger that allows setting custom intervals and units. * */
44+
public ThrottlingLogger(
45+
Logger delegate, double rateLimit, double throttledRateLimit, TimeUnit rateTimeUnit) {
46+
this(delegate, Clock.getDefault(), rateLimit, throttledRateLimit, rateTimeUnit);
47+
}
48+
3849
// visible for testing
3950
ThrottlingLogger(Logger delegate, Clock clock) {
51+
this(delegate, clock, DEFAULT_RATE_LIMIT, DEFAULT_THROTTLED_RATE_LIMIT, DEFAULT_RATE_TIME_UNIT);
52+
}
53+
54+
ThrottlingLogger(
55+
Logger delegate,
56+
Clock clock,
57+
double rateLimit,
58+
double throttledRateLimit,
59+
TimeUnit rateTimeUnit) {
4060
this.delegate = delegate;
61+
this.rateLimit = rateLimit;
62+
this.throttledRateLimit = throttledRateLimit;
63+
this.rateTimeUnit = rateTimeUnit;
4164
this.fastRateLimiter =
42-
new RateLimiter(RATE_LIMIT / rateTimeUnit.toSeconds(1), RATE_LIMIT, clock);
65+
new RateLimiter(this.rateLimit / this.rateTimeUnit.toSeconds(1), this.rateLimit, clock);
4366
this.throttledRateLimiter =
4467
new RateLimiter(
45-
THROTTLED_RATE_LIMIT / rateTimeUnit.toSeconds(1), THROTTLED_RATE_LIMIT, clock);
68+
this.throttledRateLimit / this.rateTimeUnit.toSeconds(1),
69+
this.throttledRateLimit,
70+
clock);
4671
}
4772

4873
/** Log a message at the given level. */
@@ -69,9 +94,15 @@ public void log(Level level, String message, @Nullable Throwable throwable) {
6994

7095
if (throttled.compareAndSet(false, true)) {
7196
// spend the balance in the throttled one, so that it starts at zero.
72-
throttledRateLimiter.trySpend(THROTTLED_RATE_LIMIT);
73-
delegate.log(
74-
level, "Too many log messages detected. Will only log once per minute from now on.");
97+
throttledRateLimiter.trySpend(throttledRateLimit);
98+
String timeUnitString = rateTimeUnit.toString().toLowerCase(Locale.ROOT);
99+
String throttleMessage =
100+
String.format(
101+
Locale.ROOT,
102+
"Too many log messages detected. Will only log %.0f time(s) per %s from now on.",
103+
throttledRateLimit,
104+
timeUnitString.substring(0, timeUnitString.length() - 1));
105+
delegate.log(level, throttleMessage);
75106
doLog(level, message, throwable);
76107
}
77108
}

‎sdk/common/src/test/java/io/opentelemetry/sdk/internal/ThrottlingLoggerTest.java

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import io.opentelemetry.sdk.common.Clock;
1616
import io.opentelemetry.sdk.testing.time.TestClock;
1717
import java.time.Duration;
18+
import java.util.concurrent.TimeUnit;
1819
import java.util.logging.Level;
1920
import java.util.logging.Logger;
2021
import org.junit.jupiter.api.Test;
@@ -45,6 +46,23 @@ void delegation() {
4546
.isSameAs(throwable);
4647
}
4748

49+
@Test
50+
void delegationCustom() {
51+
ThrottlingLogger logger = new ThrottlingLogger(realLogger, 10, 2, TimeUnit.HOURS);
52+
53+
logger.log(Level.WARNING, "oh no!");
54+
logger.log(Level.INFO, "oh yes!");
55+
RuntimeException throwable = new RuntimeException();
56+
logger.log(Level.SEVERE, "secrets", throwable);
57+
58+
logs.assertContains(loggingEvent -> loggingEvent.getLevel().equals(WARN), "oh no!");
59+
logs.assertContains(loggingEvent -> loggingEvent.getLevel().equals(INFO), "oh yes!");
60+
assertThat(
61+
logs.assertContains(loggingEvent -> loggingEvent.getLevel().equals(ERROR), "secrets")
62+
.getThrowable())
63+
.isSameAs(throwable);
64+
}
65+
4866
@Test
4967
void logsBelowLevelDontCount() {
5068
ThrottlingLogger logger =
@@ -76,7 +94,26 @@ void fiveInAMinuteTriggersLimiting() {
7694
assertThat(logs.getEvents()).hasSize(7);
7795
logs.assertDoesNotContain("oh no I should be suppressed!");
7896
logs.assertContains(
79-
"Too many log messages detected. Will only log once per minute from now on.");
97+
"Too many log messages detected. Will only log 1 time(s) per minute from now on.");
98+
logs.assertContains("oh no I should trigger suppression!");
99+
}
100+
101+
@Test
102+
void tenInAnHourTriggersLimiting() {
103+
Clock clock = TestClock.create();
104+
ThrottlingLogger logger = new ThrottlingLogger(realLogger, clock, 10, 2, TimeUnit.HOURS);
105+
106+
for (int i = 0; i < 10; i++) {
107+
logger.log(Level.WARNING, "oh no!");
108+
}
109+
110+
logger.log(Level.WARNING, "oh no I should trigger suppression!");
111+
logger.log(Level.WARNING, "oh no I should be suppressed!");
112+
113+
assertThat(logs.getEvents()).hasSize(12);
114+
logs.assertDoesNotContain("oh no I should be suppressed!");
115+
logs.assertContains(
116+
"Too many log messages detected. Will only log 2 time(s) per hour from now on.");
80117
logs.assertContains("oh no I should trigger suppression!");
81118
}
82119

@@ -130,7 +167,7 @@ void afterAMinuteLetOneThrough() {
130167
logs.assertDoesNotContain("oh no I should be suppressed!");
131168
logs.assertContains("oh no I should trigger suppression!");
132169
logs.assertContains(
133-
"Too many log messages detected. Will only log once per minute from now on.");
170+
"Too many log messages detected. Will only log 1 time(s) per minute from now on.");
134171

135172
clock.advance(Duration.ofMillis(60_001));
136173
logger.log(Level.WARNING, "oh no!");
@@ -145,6 +182,41 @@ void afterAMinuteLetOneThrough() {
145182
assertThat(logs.getEvents().get(8).getMessage()).isEqualTo("oh no!");
146183
}
147184

185+
@Test
186+
void afterAnHourLetTwoThrough() {
187+
TestClock clock = TestClock.create();
188+
ThrottlingLogger logger = new ThrottlingLogger(realLogger, clock, 10, 2, TimeUnit.HOURS);
189+
190+
for (int i = 0; i < 10; i++) {
191+
logger.log(Level.WARNING, "oh no!");
192+
}
193+
194+
logger.log(Level.WARNING, "oh no I should trigger suppression!");
195+
logger.log(Level.WARNING, "oh no I should be suppressed!");
196+
197+
assertThat(logs.getEvents()).hasSize(12);
198+
logs.assertDoesNotContain("oh no I should be suppressed!");
199+
logs.assertContains("oh no I should trigger suppression!");
200+
logs.assertContains(
201+
"Too many log messages detected. Will only log 2 time(s) per hour from now on.");
202+
203+
clock.advance(Duration.ofMinutes(61));
204+
logger.log(Level.WARNING, "oh no!");
205+
logger.log(Level.WARNING, "oh no!");
206+
logger.log(Level.WARNING, "oh no I should be suppressed!");
207+
assertThat(logs.getEvents()).hasSize(14);
208+
assertThat(logs.getEvents().get(13).getMessage()).isEqualTo("oh no!");
209+
210+
clock.advance(Duration.ofMinutes(61));
211+
logger.log(Level.WARNING, "oh no!");
212+
logger.log(Level.WARNING, "oh no!");
213+
logger.log(Level.WARNING, "oh no I should be suppressed!");
214+
assertThat(logs.getEvents()).hasSize(16);
215+
assertThat(logs.getEvents().get(15).getMessage()).isEqualTo("oh no!");
216+
217+
logs.assertDoesNotContain("oh no I should be suppressed!");
218+
}
219+
148220
@Test
149221
void allowOnlyOneLogPerMinuteAfterSuppression() {
150222
TestClock clock = TestClock.create();
@@ -181,4 +253,45 @@ void allowOnlyOneLogPerMinuteAfterSuppression() {
181253
assertThat(logs.getEvents()).hasSize(8);
182254
assertThat(logs.getEvents().get(7).getMessage()).isEqualTo("allowed 1");
183255
}
256+
257+
@Test
258+
void allowOnlyTwoLogPerHourAfterSuppression() {
259+
TestClock clock = TestClock.create();
260+
ThrottlingLogger logger = new ThrottlingLogger(realLogger, clock, 10, 2, TimeUnit.HOURS);
261+
262+
for (int i = 0; i < 10; i++) {
263+
logger.log(Level.WARNING, "oh no!");
264+
}
265+
266+
logger.log(Level.WARNING, "oh no I should trigger suppression!");
267+
logger.log(Level.WARNING, "oh no I should be suppressed!");
268+
269+
assertThat(logs.getEvents()).hasSize(12);
270+
271+
clock.advance(Duration.ofMinutes(10));
272+
logger.log(Level.WARNING, "suppression 1");
273+
clock.advance(Duration.ofMinutes(10));
274+
logger.log(Level.WARNING, "suppression 2");
275+
clock.advance(Duration.ofMinutes(10));
276+
clock.advance(Duration.ofSeconds(1));
277+
logger.log(Level.WARNING, "allowed 1");
278+
clock.advance(Duration.ofMinutes(10));
279+
logger.log(Level.WARNING, "suppression 3");
280+
clock.advance(Duration.ofMinutes(10));
281+
logger.log(Level.WARNING, "suppression 4");
282+
clock.advance(Duration.ofMinutes(10));
283+
clock.advance(Duration.ofSeconds(1));
284+
logger.log(Level.WARNING, "allowed 2");
285+
286+
logs.assertDoesNotContain("suppression 1");
287+
logs.assertDoesNotContain("suppression 2");
288+
logs.assertDoesNotContain("suppression 3");
289+
logs.assertDoesNotContain("suppression 4");
290+
logs.assertContains("allowed 1");
291+
logs.assertContains("allowed 2");
292+
293+
assertThat(logs.getEvents()).hasSize(14);
294+
assertThat(logs.getEvents().get(12).getMessage()).isEqualTo("allowed 1");
295+
assertThat(logs.getEvents().get(13).getMessage()).isEqualTo("allowed 2");
296+
}
184297
}

0 commit comments

Comments
 (0)