Code review sweep (run 25199276408)#18475
Merged
Merged
Conversation
Automated code review of instrumentation/logback/logback-mdc-1.0/library.
Automated code review of instrumentation/micrometer/micrometer-1.5/javaagent.
Automated code review of instrumentation/micrometer/micrometer-1.5/testing.
Automated code review of instrumentation/mongo/mongo-3.1/javaagent.
Automated code review of instrumentation/mongo/mongo-4.0/javaagent.
…version The Span Attribute Assertions guidance in the knowledge base targets spans, where hasTotalAttributeCount(0) is the preferred form for zero-attribute checks. Metric point assertions do not have hasTotalAttributeCount and point.hasAttributes(Attributes.empty()) reads more clearly than the no-arg point.hasAttributesSatisfyingExactly() form. Update the knowledge base to make this scope explicit and revert the rewrites in the micrometer-1.5 abstract tests.
trask
approved these changes
May 1, 2026
This reverts commit 3665b8d.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated code review sweep walked the following modules in order
and stopped after accumulating at least 10 modified files:
logback-appender-1.0:testinglogback-mdc-1.0:javaagentlogback-mdc-1.0:librarylogback-mdc-1.0:testingmethods:javaagentmicrometer-1.5:javaagentmicrometer-1.5:librarymicrometer-1.5:testingmongo-3.1:javaagentmongo-3.1:librarymongo-3.1:testingmongo-3.7:javaagentmongo-4.0:javaagentModule:
logback-appender-1.0:testingModule path:
instrumentation/logback/logback-appender-1.0/testingSummary
No safe repository-guideline fixes were applied. Reviewed
instrumentation/logback/logback-appender-1.0/testingand validated the modulemetadata.yamlentries against declarative config usage; no deterministic changes were needed.Applied Changes
No safe automated changes were applied.
Module:
logback-mdc-1.0:javaagentModule path:
instrumentation/logback/logback-mdc-1.0/javaagentSummary
No safe fixes were applied. A potential
assertInverse.set(true)muzzle-convention fix was attempted and reverted because:instrumentation:logback:logback-mdc-1.0:javaagent:muzzleshowed the instrumentation still passes outside the declared range.Applied Changes
No safe automated changes were applied.
Unresolved Items
File:
build.gradle.ktsReason: The Gradle muzzle convention prefers `assertInverse.set(true)` for bounded `versions.set(...)` ranges, but adding it caused inverse muzzle failures for logback versions outside `[1.0.0,1.2.3]` (for example `1.2.7`, `1.4.2`, `1.4.7`, and `1.5.32`). This needs a manual version-boundary decision, such as widening the supported range or adding a validated matcher/fail strategy, rather than an automatic review fix.
Module:
logback-mdc-1.0:libraryModule path:
instrumentation/logback/logback-mdc-1.0/librarySummary
Applied safe repository-guideline fixes under
instrumentation/logback/logback-mdc-1.0/libraryand committed them inf8f1bba4.Applied Changes
Nullability
File:
OpenTelemetryAppender.java:28Change: Annotated `MDC_MAP_FIELD` with `@Nullable`.
Reason: `knowledge/general-rules.md` nullability correctness requires fields to be annotated `@Nullable` when they can hold `null`; this field is set to `null` when reflective lookup fails.
Style
File:
LogbackTest.java:19Change: Made the `@RegisterExtension` field `testing` `final`.
Reason: `docs/contributing/style-guide.md` says fields should be `final` where possible; this extension field is assigned once.
File:
LogbackWithBaggageTest.java:15Change: Made the `@RegisterExtension` field `testing` `final`.
Reason: `docs/contributing/style-guide.md` says fields should be `final` where possible; this extension field is assigned once.
File:
LogbackWithLoggingKeysTest.java:15Change: Made the `@RegisterExtension` field `testing` `final`.
Reason: `docs/contributing/style-guide.md` says fields should be `final` where possible; this extension field is assigned once.
Module:
logback-mdc-1.0:testingModule path:
instrumentation/logback/logback-mdc-1.0/testingSummary
No safe repository-guideline fixes were applied under
instrumentation/logback/logback-mdc-1.0/testing; the scoped files already follow the loaded style, testing, metadata, and Gradle review guidance.Applied Changes
No safe automated changes were applied.
Module:
methods:javaagentModule path:
instrumentation/methods/javaagentSummary
No safe repository-guideline fixes were found under
instrumentation/methods/javaagent; no files were changed.Applied Changes
No safe automated changes were applied.
Module:
micrometer-1.5:javaagentModule path:
instrumentation/micrometer/micrometer-1.5/javaagentSummary
Applied and committed one safe repository-guideline fix in
micrometer-1.5javaagent.Applied Changes
Style
File:
TimeUnitParser.java:24Change: Changed the JUL `logger` to use `TimeUnitParser.class.getName()` and removed the now-unused `OpenTelemetryMeterRegistry` import.
Reason: `knowledge/general-rules.md` says javaagent runtime logging should preserve local module/class logger identity; using another class name obscured the warning source.
Module:
micrometer-1.5:libraryModule path:
instrumentation/micrometer/micrometer-1.5/librarySummary
No safe repository-guideline fixes were found in
instrumentation/micrometer/micrometer-1.5/library; no files were changed.Applied Changes
No safe automated changes were applied.
Module:
micrometer-1.5:testingModule path:
instrumentation/micrometer/micrometer-1.5/testingSummary
Applied and committed safe review fixes for
micrometer-1.5testing: replacedAttributes.empty()metric-point assertions with exact zero-attribute assertions and removed now-unused imports.Applied Changes
Testing
File:
AbstractFunctionTimerTest.java:206Change: Replaced `hasAttributes(Attributes.empty())` with `hasAttributesSatisfyingExactly()` and removed the unused `Attributes` import.
Reason: `testing-general-patterns.md` prefers exact attribute assertions over `hasAttributes(...)`; for metric point assertions, zero-argument `hasAttributesSatisfyingExactly()` is the supported exact zero-attribute form.
File:
AbstractLongTaskTimerHistogramTest.java:58Change: Replaced zero-attribute `hasAttributes(Attributes.empty())` checks with `hasAttributesSatisfyingExactly()` and removed the unused `Attributes` import.
Reason: `testing-general-patterns.md` prefers exact attribute assertions over `hasAttributes(...)`; for metric point assertions, zero-argument `hasAttributesSatisfyingExactly()` is the supported exact zero-attribute form.
File:
AbstractTimerTest.java:118Change: Replaced zero-attribute `hasAttributes(Attributes.empty())` checks with `hasAttributesSatisfyingExactly()` and removed the unused `Attributes` import.
Reason: `testing-general-patterns.md` prefers exact attribute assertions over `hasAttributes(...)`; for metric point assertions, zero-argument `hasAttributesSatisfyingExactly()` is the supported exact zero-attribute form.
Unresolved Items
File:
AbstractGaugeTest.javaReason: `testWeakRefGauge()` still declares `InterruptedException, TimeoutException`; changing it to single `Exception` fails Error Prone `InterruptedExceptionSwallowed`, and wrapping solely to narrow the `@Test` throws clause is disallowed by `testing-general-patterns.md`.
Module:
mongo-3.1:javaagentModule path:
instrumentation/mongo/mongo-3.1/javaagentSummary
Applied and committed one safe review fix for
mongo-3.1javaagent.Applied Changes
Javaagent
File:
MongoClientInstrumentationModule.java:57Change: Moved `MongoClientAdvice` inside `MongoClientOptionsBuilderInstrumentation`, changed advice lookup to `getClass().getName() + "$MongoClientAdvice"`, and narrowed the nested advice class visibility.
Reason: Repository `javaagent-advice-patterns.md` requires advice classes to be nested inside the `TypeInstrumentation` that applies them; the transform guideline prefers `getClass().getName()` for nested advice references, and Error Prone `EffectivelyPrivate` requires avoiding `public` on an effectively private nested class.
Module:
mongo-3.1:libraryModule path:
instrumentation/mongo/mongo-3.1/librarySummary
No safe repository-guideline fixes were found in
instrumentation/mongo/mongo-3.1/library; no files were changed.Applied Changes
No safe automated changes were applied.
Module:
mongo-3.1:testingModule path:
instrumentation/mongo/mongo-3.1/testingSummary
No safe repository-guideline fixes were applied under
instrumentation/mongo/mongo-3.1/testing; reviewed files already match applicable style, testing, Gradle, and mandatorymetadata.yamlguidance.Applied Changes
No safe automated changes were applied.
Module:
mongo-3.7:javaagentModule path:
instrumentation/mongo/mongo-3.7/javaagentSummary
No safe repository-guideline fixes were found in
instrumentation/mongo/mongo-3.7/javaagent; the reviewed source, test, Gradle, andmetadata.yamlpatterns already match the applicable Javaagent, testing, Gradle, singleton, classloader matcher, and metadata rules.Applied Changes
No safe automated changes were applied.
Module:
mongo-4.0:javaagentModule path:
instrumentation/mongo/mongo-4.0/javaagentSummary
Applied one safe repository-guideline fix and committed it as
e110aec8(Review fixes for mongo-4.0 javaagent).Applied Changes
Build
File:
build.gradle.kts:25Change: Removed unused `testImplementation("de.flapdoodle.embed:de.flapdoodle.embed.mongo:1.50.5")`.
Reason: `gradle-conventions.md` says to remove unused or redundant dependencies; the module tests use `mongo-common:testing`, whose shared base uses Testcontainers, and no source in this module references Flapdoodle.
Download code review diagnostics