Skip to content

fix(azure_storage): keep the DataLakeServiceClient alive until its TTL elapses - #43082

Merged
yucheng-berri merged 15 commits into
mainfrom
litellm_azure_storage_client_ttl
Oct 1, 2026
Merged

yucheng-berri merged 15 commits into
mainfrom
litellm_azure_storage_client_ttl

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • azure_storage account-key uploads close their Data Lake client on almost every call
  • The expiry check is inverted: a client inside its TTL is treated as expired
  • Concurrent flushes then run on a closed client and fail with AuthenticationFailed
  • Those failures are swallowed, so audit records are silently dropped

How it solves it:

  • Close and rebuild the client only once its TTL has actually elapsed
  • Inject the clock so the TTL boundary is testable without patching time
  • Ship deterministic tests/integration/observability/ cells against a local Data Lake sink

User Flow

Before: a proxy admin with success_callback: [azure_storage] and AZURE_STORAGE_ACCOUNT_KEY set finds only part of their traffic in storage

  1. They send 40 POST https://litellm-domain/v1/chat/completions calls, all return 200
  2. The proxy log shows azure.core.exceptions.ClientAuthenticationError: (AuthenticationFailed) for many of them
  3. Listing the day's folder in the storage container shows roughly half of the 40 JSON files

After: every successful call lands in storage

  1. They send the same 40 POST https://litellm-domain/v1/chat/completions calls, all return 200
  2. The proxy log shows no Azure authentication errors
  3. Listing the day's folder in the storage container shows all 40 JSON files

Linear ticket

Resolves LIT-8601

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Two proofs, each base vs head. The reproduction runs against a real Azure Data Lake Gen2 account (isolated resource group, account key in the proxy env only). The deterministic one is the integration audit for LIT-8601 at this PR's tip: real proxy with two workers, Postgres and Redis, scripted upstream, local TLS sink speaking the Data Lake REST surface the SDK emits, no provider call, no credential. Runner uv run --no-sync python tests/integration/run.py over tests/integration/observability/test_azure_storage_client_ttl.py and test_azure_storage_chaos.py, same --seed and --order-seed on every run, run ids phase12/0b7c9ee496/base, phase12/0b7c9ee496/head1, phase12/0b7c9ee496/head2. After the fresh merge of main at f37a0c4 (which brings the LIT-9079 filename fix and its test_azure_storage_file_names.py cell), the same selection plus that cell was rerun on the merged tip: 16 passed, 0 skipped, in 540s

Observable for the deterministic proof: TCP connections the sink accepts per upload. One live DataLakeServiceClient pools one blob plus one dfs connection per proxy worker; a client rebuilt per upload opens a fresh pair every time

Before (merge base 501ef23, unfixed code)

Reproduction against real Azure

  1. One AzureBlobStorageLogger with the account key, 40 uploads at once (the same shape as the ticket's controlled test): 20/40 objects land, 20 AssertionErrors from the aiohttp transport because the next caller closes the shared client mid request
  2. Through a running proxy the loss does not reproduce: 1 worker, 2 workers with success plus failure callbacks, a 120 request burst, Entra ID without an account key and the v1.97.0 checkout all land 100 percent, because the queue flush holds a lock and uploads one payload at a time. There the inverted check only rebuilds the client on every upload, which the sink counts below

Deterministic audit, 15 azure nodes, 13 passed, 2 failed, 0 skipped

  1. tests/integration/observability/test_azure_storage_client_ttl.py::test_every_surface_lands_once_and_the_client_is_reused_across_uploads: 18 uploads across chat, messages and responses (stream and non stream, openai, anthropic and httpx clients) all land by id, then AssertionError: 36 sink connections for 18 uploads
  2. tests/integration/observability/test_azure_storage_chaos.py::test_slow_sink_lands_every_id_once_without_deadlock: 36 concurrent uploads land once each, then AssertionError: 72 sink connections for 36 uploads
  3. The other 13 azure nodes pass, so caller status, upstream receipt, failure payloads, /v1/files sibling, the disabled callback and the restart bound are unchanged from base

After (0b7c9ee, test set unchanged at f37a0c4)

Reproduction against real Azure

  1. The same 40 concurrent uploads on one logger: 40/40 objects land, zero transport errors
  2. Entra ID without an account key, through a two-worker proxy, lands 60/60 on both legs (the bearer path never shared the bug)

Deterministic audit, 15 azure nodes, 15 passed, 0 skipped, run twice with identical collected and passed sets; 16 of 16 at the merged tip f37a0c4

  1. Both nodes above pass: 4 sink connections for 18 uploads and 4 for 36 across two workers
  2. All 15 azure nodes pass in head1 and again in head2
cell node base head1 head2
H1-H7 six surfaces, reuse bound test_azure_storage_client_ttl.py::test_every_surface_lands_once_and_the_client_is_reused_across_uploads FAIL (reuse) PASS PASS
H8 success_callback test_azure_storage_client_ttl.py::test_success_callback_mode_uploads_success_and_skips_failure PASS PASS PASS
H9 failure_callback, upstream 500 test_azure_storage_client_ttl.py::test_failure_callback_mode_uploads_only_failures PASS PASS PASS
S1 sink 403 test_azure_storage_client_ttl.py::test_sink_403_keeps_the_caller_and_proxy_healthy PASS PASS PASS
S2 sink 404 test_azure_storage_client_ttl.py::test_sink_404_keeps_the_caller_and_proxy_healthy PASS PASS PASS
S3 upstream 401 test_azure_storage_client_ttl.py::test_upstream_401_reaches_the_caller_and_lands_as_a_failure_payload PASS PASS PASS
S4 unknown model test_azure_storage_client_ttl.py::test_unknown_model_lands_as_a_failure_payload PASS PASS PASS
S6 file system setting missing test_azure_storage_client_ttl.py::test_missing_file_system_setting_fails_the_callback_init_and_keeps_the_proxy_serving PASS PASS PASS
E2 repeated identical requests test_azure_storage_client_ttl.py::test_repeated_identical_requests_each_land_exactly_once PASS PASS PASS
E4 /v1/files sibling test_azure_storage_client_ttl.py::test_files_upload_to_azure_storage_sibling_path_is_unchanged PASS PASS PASS
E5 callback disabled test_azure_storage_client_ttl.py::test_disabled_callback_opens_no_sink_connection PASS PASS PASS
C1 sink outage mid burst, C5 readiness test_azure_storage_chaos.py::test_sink_outage_mid_burst_loses_only_the_outage_window_and_recovers_exactly_once PASS PASS PASS
C2/E3 slow sink, 36 concurrent test_azure_storage_chaos.py::test_slow_sink_lands_every_id_once_without_deadlock FAIL (reuse) PASS PASS
C3 kill one worker test_azure_storage_chaos.py::test_killing_one_worker_keeps_the_other_serving_and_uploading PASS PASS PASS
C4 restart the proxy before the queue flushes test_azure_storage_chaos.py::test_restarting_the_proxy_before_the_queue_flushes_bounds_the_loss_to_the_unflushed_queue_and_recovers PASS PASS PASS

Unit regression: test_service_client_is_reused_until_its_ttl_elapses, test_service_client_is_replaced_once_its_ttl_elapses and test_service_client_is_replaced_at_the_exact_ttl_boundary in tests/unit/integrations/azure_storage/test_azure_storage.py. Mutations: flip <= back to > and the first fails, drop the TTL branch and the second fails, change <= to < and the third fails

Not run as an integration cell: E1 exact TTL boundary through a running proxy (the clock is a constructor argument the proxy does not expose and the TTL is a module constant, so it stays at the unit level). S5 Entra ID and the real upload path were run live as listed above and are out of the deterministic audit's scope by design; the earlier opt-in tests/e2e/logging cells and their e2e-dev dependency were dropped from this PR on request, so pyproject.toml and uv.lock are untouched

Reviewer must know before approving

  • Account-key uploads reuse one Data Lake client for the whole TTL (1 hour, _DEFAULT_TTL_FOR_HTTPX_CLIENTS)
    • before: every upload closed the previous client and opened a new one (two TCP connections per upload), and a flush overlapping with another caller on the same logger could run on a closed client and drop its record
    • after: one client per proxy worker is kept until its TTL elapses, then closed and rebuilt; connection count drops from 2 per upload to 2 per worker per TTL, the object names and JSON bodies are unchanged
    • approval: pending, yucheng-berri in the Devin session linked below
  • AzureBlobStorageLogger gains a clock constructor argument
    • before: the TTL was read from time.time() directly
    • after: clock: Callable[[], float] = time.time is injectable; the proxy never passes it, so runtime behavior is identical and only tests use it
    • approval: pending, yucheng-berri in the Devin session linked below

Type

🐛 Bug Fix
✅ Test

Caveats (if any)

Medium

  • CircleCI has not started a pipeline for this branch at any push, so integration-extensions is unconfirmed at f37a0c4
    • Sibling same-repo branches pushed the same day do get ci/circleci: integration-* statuses
    • The azure selection was run locally three times as shown above
  • The customer's exact AuthenticationFailed text was not observed in any run; what reproduces is the shared-client transport failure and the resulting missing objects

Low

  • At the exact TTL boundary a flush already in flight still shares the client being closed; that is once per TTL instead of every call, pre-existing base behavior, and a grace-period close is a follow-up
  • The account-key path writes {date}/{id}.json while the Entra ID path writes {id}.json at the filesystem root; pre-existing on both legs, unifying the layout is a follow-up
  • The full extensions group still carries unrelated failures on the audit box (test_redis_outage_keeps_serving_in_memory_hits, the bedrock SigV4 guardrail node, test_passthrough_worker_sigkill_leaves_sibling_serving_and_logging) and one pre-existing BUG: skip; the azure nodes are deterministic and skip free

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Link to Devin session: https://app.devin.ai/sessions/5c920e07636044cebaef8e7a723300fe
Open in Devin Desktop: https://app.devin.ai/desktop/session/5c920e07636044cebaef8e7a723300fe?variant=devin
Requested by: @yucheng-berri

…L elapses

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot requested a review from a team September 24, 2026 23:36
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@CLAassistant

CLAassistant commented Sep 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ yucheng-berri
❌ devin-ai-integration[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_azure_storage_client_ttl (f37a0c4) with main (431ecd8)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Fixes Azure Storage client lifecycle management in logging.

The PR appears safe to merge; the remaining read-back scalability issue is non-blocking.

Findings

  1. P2 Unbounded Azure object listing ▶

Summary

The PR corrects the Azure Data Lake client TTL comparison, injects a clock for boundary tests, and adds deterministic integration coverage and opt-in live read-back tests.

  • Account-key uploads now reuse the service client until expiry.
  • The new read-back helper has an unbounded listing cost as the audit filesystem grows.

Reviews (5) · Last reviewed commit: "chore: merge main into litellm_azure_sto..."

Comment thread litellm/integrations/azure_storage/azure_storage.py
Comment thread tests/unit/integrations/azure_storage/test_azure_storage.py
devin-ai-integration Bot and others added 4 commits September 24, 2026 23:49
…meout

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ata Lake sink

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai please review the current head e695133

@cursor cursor Bot 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.

Stale Bugbot comment from a previous run.

…nk back

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

bugbot run

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@mateo-berri

Copy link
Copy Markdown
Contributor

@greptileai

@cursor cursor Bot 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.

Stale Bugbot comment from a previous run.

yucheng-berri and others added 2 commits September 26, 2026 23:07
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…o the unflushed queue

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
yucheng-berri and others added 2 commits September 26, 2026 23:34
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@cursor

cursor Bot commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@yucheng-berri

Copy link
Copy Markdown
Contributor

bugbot run

@yucheng-berri

Copy link
Copy Markdown
Contributor

@greptileai review latest head

@cursor cursor Bot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0b7c9ee. Configure here.

try:
return [
path.name
for path in fs_client.get_paths()

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.

P2 Unbounded Azure object listing

Each poll for one response ID lists every path in the filesystem, and the exactly-one check lists them all again. As audit records accumulate, these tests will make increasingly expensive listings and may time out even when the object was delivered. Limit the lookup to the relevant paths or date directories.

yucheng-berri and others added 2 commits October 1, 2026 01:24
…2e-dev dependency

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…_client_ttl

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

# Conflicts:
#	tests/integration/observability/_azure_storage_support.py
#	tests/unit/integrations/azure_storage/test_azure_storage.py
@yucheng-berri
yucheng-berri merged commit 54ae4c5 into main Oct 1, 2026
98 of 104 checks passed
@yucheng-berri
yucheng-berri deleted the litellm_azure_storage_client_ttl branch October 1, 2026 01:50
jan-sauer-reef added a commit to jan-sauer-reef/litellm that referenced this pull request Oct 1, 2026
…ject_key_prefix

* upstream/main: (62 commits)
  fix(guardrails): scan Responses API input in Azure Prompt Shield (BerriAI#43786)
  feat(lens): investigate sampled traces and retain batch results (BerriAI#43942)
  fix(proxy): restore pre-config-wins handling of pass-through endpoints (BerriAI#43962)
  fix(cost-map): raise baseten DeepSeek-V4.1-Flash max output to 262144 (BerriAI#43916)
  chore(cost-map): add deprecation date for anthropic claude-sonnet-4-5 (BerriAI#43898)
  chore(cost-map): add fireworks inkling priority prices from the prices api (BerriAI#43949)
  feat(guardrails): honor litellm_params.timeout in every HTTP guardrail (BerriAI#43134)
  test(e2e): typed per-test metadata for the e2e suite (BerriAI#42044)
  fix(caching): write the response-cache SET to Redis at once instead of on the post-call batch (BerriAI#43973)
  feat(ui): filter tags by name and description on the Tag Management page (BerriAI#42949)
  feat(providers): add Cortecs as an OpenAI-compatible provider (BerriAI#43872)
  feat(e2e): record each e2e test's steps, starting with ProxyClient (BerriAI#42393)
  test(ci): repair stale tests and move retired OpenAI text-completion fixtures (BerriAI#43958)
  feat(proxy): record in spend logs whether a request used a client-forwarded Anthropic OAuth token (BerriAI#43063)
  fix(azure_storage): keep the DataLakeServiceClient alive until its TTL elapses (BerriAI#43082)
  chore(deps): bump gitpython and tornado, extend diskcache osv ignore to Nov 1 (BerriAI#43961)
  fix(guardrails): treat an unknown straiker api_version as unset instead of skipping the guardrail (BerriAI#43956)
  fix(azure_storage): name Data Lake objects without base64 padding or slashes (BerriAI#43914)
  fix(grayswan): send request conversation and tool calls to post-call monitor (BerriAI#43770)
  chore(cost-map): sync openrouter prices from the models API (BerriAI#43950)
  ...
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.

3 participants