Skip to content

[cosmos] Fix ClientTelemetry static-init NPE when IMDS access is disabled #48887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lfavreli-betclic wants to merge 3 commits into from
+7 −3
Closed

[cosmos] Fix ClientTelemetry static-init NPE when IMDS access is disabled #48887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9b55a1
[cosmos] Fix ClientTelemetry static-init NPE when IMDS access disabled
lfavreli-betclic Apr 21, 2026
5a14729
Apply suggestions from code review
FabianMeiswinkel Apr 21, 2026
8e1deb4
Apply suggestion from @FabianMeiswinkel
FabianMeiswinkel Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/cosmos/azure-cosmos/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Fixed JVM `<clinit>` deadlock when multiple threads concurrently trigger Cosmos SDK class loading for the first time. - See [PR 48689](https://github.com/Azure/azure-sdk-for-java/pull/48689)
* Fixed an issue where `CustomItemSerializer` was incorrectly applied to internal SDK query pipeline structures (e.g., `OrderByRowResult`, `Document`), causing deserialization failures in ORDER BY, GROUP BY, aggregate, DISTINCT, and hybrid search queries. - See [PR 48811](https://github.com/Azure/azure-sdk-for-java/pull/48811)
* Fixed an issue where `SqlParameter` ignored the configured `CustomItemSerializer`, always using the internal default serializer instead. - See [PR 48811](https://github.com/Azure/azure-sdk-for-java/pull/48811)
* Fixed `NullPointerException` during `ClientTelemetry` static initialization when IMDS access is disabled (`COSMOS_DISABLE_IMDS_ACCESS=true`), which made every `CosmosAsyncClient` build fail with `NoClassDefFoundError`. - See [PR 48887](https://github.com/Azure/azure-sdk-for-java/pull/48887)

#### Other Changes

Expand Down
9 changes: 6 additions & 3 deletions ...cosmos/src/main/java/com/azure/cosmos/implementation/clienttelemetry/ClientTelemetry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,18 @@ private static ImplementationBridgeHelpers.CosmosClientTelemetryConfigHelper.Cos
private static final Logger logger = LoggerFactory.getLogger(ClientTelemetry.class);
private static final String USER_AGENT = Utils.getUserAgent();

// Sentinel for "not on Azure VM" or "IMDS unreachable".
// Must be declared before CACHED_METADATA so that fetchAzureVmMetadata()
// never reads a null value during class initialization (e.g. when IMDS
// access is disabled via COSMOS_DISABLE_IMDS_ACCESS).
private static final AzureVMMetadata METADATA_NOT_AVAILABLE = new AzureVMMetadata();
Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel Apr 21, 2026

Choose a reason for hiding this comment

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

Like you mentioned in the PR description - moving to static initialization block to also fix the fragile IMDS_DEFAULT_* fields - would be my preference. I can take the additional changes (adding test coverage and making this change form here if you don't wnat to spend more time on this - perfectly understandable). If you want to finish/merge the PR please let me know when these changes are added and I will quickly re-review and approve.

Copy link
Copy Markdown
Author

@lfavreli-betclic lfavreli-betclic Apr 21, 2026

Choose a reason for hiding this comment

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

Thanks a lot, @FabianMeiswinkel, for your quick review!

You have a much better overall understanding of the SDK, so I’m perfectly happy to let you take over and apply the remaining changes. I hope this initial work was helpful to you. 👌

Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel Apr 21, 2026

Choose a reason for hiding this comment

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

It was and thanks for providing such detailed repro-info - really appreciated!

Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel Apr 21, 2026

Choose a reason for hiding this comment

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

Thanks @lfavreli-betclic - i have published a PR #48888 for this and will get it merged asap.


// Cached IMDS metadata Mono. Reactor's cache() ensures:
// - The fetch executes at most once
// - All concurrent subscribers share the single result
// - The HTTP client is created and disposed within the fetch
private static final Mono<AzureVMMetadata> CACHED_METADATA = fetchAzureVmMetadata().cache();
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

Consider adding a regression test to cover the class-initialization scenario this change fixes (IMDS access disabled causing ClientTelemetry to become unloadable). There is already child-JVM test infrastructure in azure-cosmos-tests (e.g., ImplementationBridgeHelpersTest uses ProcessBuilder) that could run a small main with -DCOSMOS.DISABLE_IMDS_ACCESS=true (or COSMOS_DISABLE_IMDS_ACCESS=true) before any Cosmos classes load, then assert that a CosmosClientBuilder.buildAsyncClient() call completes without ExceptionInInitializerError/NoClassDefFoundError. This would prevent future static-field reordering regressions.

Copilot generated this review using guidance from repository custom instructions.
Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel Apr 21, 2026

Choose a reason for hiding this comment

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

+1


// Sentinel for "not on Azure VM" or "IMDS unreachable"
private static final AzureVMMetadata METADATA_NOT_AVAILABLE = new AzureVMMetadata();

// IMDS Constants
private static final String IMDS_AZURE_VM_METADATA = "http://169.254.169.254:80/metadata/instance?api-version=2020-06-01";
private static final Duration IMDS_DEFAULT_NETWORK_REQUEST_TIMEOUT = Duration.ofSeconds(5);
Expand Down
Loading