Skip to content

docs(agent-extension-api): mark ConfigProperties @Nullable where null is possible#18090

Open
zeitlinger wants to merge 4 commits into
open-telemetry:mainfrom
zeitlinger:declarative-config-nullable-sdk-config
Open

docs(agent-extension-api): mark ConfigProperties @Nullable where null is possible#18090
zeitlinger wants to merge 4 commits into
open-telemetry:mainfrom
zeitlinger:declarative-config-nullable-sdk-config

Conversation

@zeitlinger

Copy link
Copy Markdown
Member

Summary

  • Annotate config parameters and locals as @Nullable on the SPIs where a null ConfigProperties already flows through in declarative-config mode.
  • Document the declarative-config contract in the javadoc of AgentExtension.extend and BootstrapPackagesConfigurer.configure.

Context

In declarative-config mode, AutoConfigureUtil.getConfig(autoConfiguredSdk) returns null. That null already flows through AgentInstaller into:

  • BootstrapPackagesConfigurer.configure(builder, config)
  • IgnoredTypesConfigurer.configure(builder, config) (deprecated overload; AgentInstaller already substitutes EmptyConfigProperties.INSTANCE before calling it)
  • AgentExtension.extend(builder, config)

The signatures did not reflect that. This PR aligns the public contract with actual runtime behavior.

Split off from #17719 so the NullAway enablement PR does not need a runtime workaround (empty-map fallback or requireNonNull) that would silently break extensions using config == null to detect declarative mode.

No behavior change.

Test plan

  • CI green
  • No runtime behavior change — only annotations + javadoc

… is possible

In declarative-config mode, AutoConfigureUtil.getConfig(autoConfiguredSdk)
returns null. That null already flows through AgentInstaller into the
BootstrapPackagesConfigurer, IgnoredTypesConfigurer (deprecated overload),
and AgentExtension SPIs, but the signatures did not reflect it.

Annotate the affected parameters and locals as @nullable and document the
declarative-config contract in the SPI javadoc. No behavior change.

This aligns the public contract with actual runtime behavior so NullAway can
be enabled on the calling modules without needing a runtime workaround.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger zeitlinger marked this pull request as ready for review April 20, 2026 11:42
@zeitlinger zeitlinger requested a review from a team as a code owner April 20, 2026 11:43
@laurit

laurit commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Supplying null to a method that previously was never invoked with null is essentially an incompatible change. Perhaps it would be better to invoke these methods with empty properties instead?

trask added a commit to trask/opentelemetry-java-instrumentation that referenced this pull request Apr 30, 2026
Pulling main into a PR branch (e.g. trask clicking 'Update branch' on open-telemetry#18090) was being recorded as the latest substantive event, masking earlier unanswered approver questions. Filter such commits out of the substantive timeline so the author still owes a response.
trask added a commit to trask/opentelemetry-java-instrumentation that referenced this pull request May 1, 2026
The is_substantive() filter excluded "Update branch" merge commits from the latest-event determination, but treated all merge commits the same regardless of who made them, and the LLM-rendered context still surfaced them in the "Last N commits" listing and used their timestamp for "Last commit pushed".

Refine the rule to only ignore merge commits authored by someone other than the PR author (e.g. an approver clicking "Update branch" on someone else's PR, as on open-telemetry#18090). Author-driven merges of base into the PR remain substantive. Apply the same rule to the rendered commit listing and last_commit_date.
@github-actions github-actions Bot mentioned this pull request May 5, 2026
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Copilot AI review requested due to automatic review settings May 18, 2026 17:02

zeitlinger commented May 18, 2026

Copy link
Copy Markdown
Member Author

Good point — I took the safer route here. Instead of widening the non-deprecated extension SPIs to nullable, I now normalize the declarative-config case to an empty ConfigProperties before calling them.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to align javaagent extension-facing configuration nullability with declarative-config behavior, but the implementation currently normalizes nullable config to an empty config before some SPIs receive it.

Changes:

  • Marks the local SDK config in AgentInstaller as nullable.
  • Introduces an EmptyConfigProperties fallback before passing config to agent setup hooks.
  • Annotates the deprecated IgnoredTypesConfigurer.configure(..., config) parameter as @Nullable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java Updates SDK config handling and passes a normalized config into bootstrap, ignored-types, and extension configuration paths.
javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java Marks the deprecated config parameter as nullable.
Comments suppressed due to low confidence (1)

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java:180

  • This changes declarative-config mode from passing null to AgentExtension.extend to passing EmptyConfigProperties. That removes the config == null signal for custom extensions, and the built-in instrumentation loader will also forward the non-null empty config to instrumentation modules, contradicting the no-runtime-behavior-change/declarative-mode contract described for this PR.
        agentBuilder = agentExtension.extend(agentBuilder, sdkConfigOrEmpty);

@zeitlinger zeitlinger marked this pull request as draft May 18, 2026 17:08
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger zeitlinger marked this pull request as ready for review May 18, 2026 17:21
Sign up for free to 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.

4 participants