docs(agent-extension-api): mark ConfigProperties @Nullable where null is possible#18090
docs(agent-extension-api): mark ConfigProperties @Nullable where null is possible#18090zeitlinger wants to merge 4 commits into
Conversation
… 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>
|
Supplying |
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.
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.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
|
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 |
There was a problem hiding this comment.
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
AgentInstalleras nullable. - Introduces an
EmptyConfigPropertiesfallback 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.extendto passingEmptyConfigProperties. That removes theconfig == nullsignal 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);
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Summary
configparameters and locals as@Nullableon the SPIs where a nullConfigPropertiesalready flows through in declarative-config mode.AgentExtension.extendandBootstrapPackagesConfigurer.configure.Context
In declarative-config mode,
AutoConfigureUtil.getConfig(autoConfiguredSdk)returnsnull. That null already flows throughAgentInstallerinto:BootstrapPackagesConfigurer.configure(builder, config)IgnoredTypesConfigurer.configure(builder, config)(deprecated overload;AgentInstalleralready substitutesEmptyConfigProperties.INSTANCEbefore 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 usingconfig == nullto detect declarative mode.No behavior change.
Test plan