Skip to content

[demo] Add Bucket4j rate-limiter instrumentation - #11903

Closed
dougqh wants to merge 2 commits into
masterfrom
dougqh/bucket4j-perf-demo
Closed

[demo] Add Bucket4j rate-limiter instrumentation#11903
dougqh wants to merge 2 commits into
masterfrom
dougqh/bucket4j-perf-demo

Conversation

@dougqh

Copy link
Copy Markdown
Contributor

⚠️Demo artifact — do not merge. Will be closed after the talk.

This PR exists to demo the AI-assisted performance-review rubric for the Java build-team talk. The instrumentation is intentionally written with several hot-path anti-patterns in Bucket4jDecorator.onConsume (which runs on every Bucket#tryConsume):

  • per-call InstrumenterConfig.get().isIntegrationEnabled(singleton(...)) — config lookup + a fresh set allocation every call
  • Arrays.stream(...).filter(...).findFirst() in the hot path
  • Objects.hash(...) — varargs array + boxing every call
  • eager LOGGER.debug("..." + ... + bucket) string building

…alongside a benign one-time use of the sameObjects.hash pattern in cold static init (buildKnownLimitKeys).

The point of the demo: the reviewer should flag the hot-path costs and stay silent on the cold one — situational awareness, not a linter.

🤖 Generated with Claude Code

dougqhand others added 2 commits July 9, 2026 14:45
Traces bucket4j rate-limit checks: one span per Bucket#tryConsume with
tokens/consumed tags and a rate tier.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dd-octo-sts

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

SuiteStatus
Startup🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
ScenarioCandidatemasterΔ (95% CI of mean)
startup:insecure-bank:iast:Agent14.77 s14.66 s[-0.1%; +1.6%] (no difference)
startup:insecure-bank:tracing:Agent13.60 s13.70 s[-1.5%; -0.1%] (maybe better)
startup:petclinic:appsec:Agent16.79 s16.80 s[-1.1%; +0.9%] (no difference)
startup:petclinic:iast:Agent16.86 s16.92 s[-1.4%; +0.6%] (no difference)
startup:petclinic:profiling:Agent16.40 s16.88 s[-7.2%; +1.5%] (no difference)
startup:petclinic:sca:Agent16.94 s16.82 s[+0.1%; +1.4%] (maybe worse)
startup:petclinic:tracing:Agent16.14 s16.21 s[-1.3%; +0.5%] (no difference)

Commit:62abebcb · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@dougqhdougqh left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Claude perf rubric...

Perf review (rubric: perf-review-checks.md)

Scope: Bucket4jDecorator.onConsume — runs on every Bucket#tryConsume call (tracing hot path; multiplier = per-call × calls/sec).

Confirmed findings

1. Per-call config lookup + Set allocation

InstrumenterConfig.get().isIntegrationEnabled(singleton("bucket4j-tier"), true)

Collections.singleton(...) allocates a new SingletonSet every call, plus a config lookup, for a value that doesn't change per-call.

  • Rubric check #2 (repeat work on invariant input) — SEV-2/3
  • Fix: hoist to a static final boolean (or cache in a field) computed once.

2. Arrays.stream(...).filter(...).findFirst() in the hot path
Builds a Stream pipeline (Stream + Spliterator + pipeline stages + captured lambda) every call just to find the first threshold ≥ tokens.

  • Rubric check #1/#5 (per-call allocation + unnecessary indirection) — SEV-3
  • Fix: plain for loop over TIER_THRESHOLDS, no Stream.

3. Objects.hash(bucket, tokens, consumed) in onConsume
Varargs Object[] allocation + boxing of tokens (long) and consumed (boolean) on every call, unconditionally.

  • Rubric check J9 — SEV-2/3
  • Fix: datadog.trace.util.HashingUtils (no boxing, no array).

4. Eager string concatenation in LOGGER.debug(...)

LOGGER.debug("bucket4j tryConsume tokens=" + tokens + " consumed=" + consumed + " bucket=" + bucket);

Builds the string unconditionally, even when debug logging is disabled.

  • Rubric check #2/J10 — SEV-2/3
  • Fix: SLF4J parameterized form, or guard with isDebugEnabled().

Correctly not flagged

DEFAULT_LIMIT_KEY = Objects.hash("default", 100L) — same Objects.hash pattern as #3, but this one runs once at class-init (cold path). Per the rubric's precision-over-recall posture, staying silent here.

Posted per the AI-review rubric demo — no action required for this closed/demo PR.

@dougqhdougqh left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perf review per perf-review-checks.md rubric — 4 hot-path findings inline below. onConsume runs on every Bucket#tryConsume call, so multiplier = per-call × calls/sec.


public void onConsume(AgentSpan span, Bucket bucket, long tokens, boolean consumed) {
long tier = -1L;
if (InstrumenterConfig.get().isIntegrationEnabled(singleton("bucket4j-tier"), true)) {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per-call config lookup + Set allocation.Collections.singleton("bucket4j-tier") allocates a new SingletonSet every call, plus a config lookup, for a value that doesn't change per-call.

long tier = -1L;
if (InstrumenterConfig.get().isIntegrationEnabled(singleton("bucket4j-tier"), true)) {
tier =
Arrays.stream(TIER_THRESHOLDS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream pipeline in the hot path.Arrays.stream(...).filter(...).findFirst() builds a Stream + Spliterator + pipeline stages + captured lambda every call just to find the first threshold ≥ tokens.

span.setTag("bucket4j.tier", tier);
}

int metricKey = Objects.hash(bucket, tokens, consumed);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objects.hash varargs/boxing on the hot path.Objects.hash(bucket, tokens, consumed) allocates an Object[] and boxes tokens/consumed on every call, unconditionally.

  • Rubric check J9 — SEV-2/3
  • Fix: datadog.trace.util.HashingUtils (no boxing, no array).

(Not flagging DEFAULT_LIMIT_KEY = Objects.hash("default", 100L) above at line 27 — that one runs once at class-init, cold path.)

}

LOGGER.debug(
"bucket4j tryConsume tokens=" + tokens + " consumed=" + consumed + " bucket=" + bucket);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eager string concatenation in LOGGER.debug(...). The string is built unconditionally (StringBuilder + bucket.toString()), even when debug logging is disabled.

  • Rubric check First PR - Implements the Opentracing API #2/J10 — SEV-2/3
  • Fix: SLF4J parameterized form LOGGER.debug("bucket4j tryConsume tokens={} consumed={} bucket={}", tokens, consumed, bucket), or guard with isDebugEnabled().

private static final long[] TIER_THRESHOLDS = {1L, 10L, 100L, 1_000L, 10_000L};

// stable id for the default bandwidth profile; computed once at class load
private static final int DEFAULT_LIMIT_KEY = Objects.hash("default", 100L);

@dougqhdougqhJul 9, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objects.hash called - but only once during class initialization, so not alerted.

@dougqh

Copy link
Copy Markdown
ContributorAuthor

This just intended as a test / demo of the perf review skill. It has fulfilled that purpose.

@dougqhdougqh closed this Jul 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@dougqh