Make dotnet-eng-status deployment notifications best-effort (fix intermittent 500)

[missymessa]

Problem

The Helix deploy pipelines (dotnet-helix-service-deploy, dotnet-helix-machines deploy) intermittently fail their "Notify dotnet-eng-status/start" step with an HTTP 500, e.g. build 3024453:

POST https://dotneteng-status-staging.azurewebsites.net/api/deployment/dotnet-helix-service/2026071601/start -> (500) Internal Server Error

The 500 comes from DeploymentController.MarkStart in the DotNetStatus (dotnet-eng-status) app, not from Helix. Recording a deployment annotation is best-effort telemetry, but the controller had two problems that let any hiccup become a 500 that shows up as a red step in the deploy pipeline:

1. The Grafana annotation call was wrapped in ExponentialRetry.RetryAsync(..., isRetryable: e => true), which retries every exception (including non-transient 4xx from EnsureSuccessStatusCode()) up to 10 times and then rethrows.
2. Neither the retried Grafana call nor the follow-up table upsert was wrapped in a try/catch, so any rethrown/unretried exception became an unhandled 500.

Fix

• Narrow the retry predicate to a new IsTransientFailure helper: retry only network errors, timeouts, and 5xx responses (HttpRequestException with StatusCode null or >= 500, or TaskCanceled/OperationCanceled). Non-transient failures now fail fast instead of being retried 10× and rethrown.
• Wrap the MarkStart and MarkEnd bodies in try/catch that logs the error and returns NoContent(). Best-effort deployment notifications never fail the calling pipeline again.
• MarkEnd now logs and returns NoContent() instead of NotFound() when no matching start record exists (a missing/failed start notification should not turn the end notification into a 404).

Notes

• Behavior on the success path is unchanged.
• Observability on the staging app is currently broken (App Insights instrumentation key points at an empty component), so the exact failing dependency (Grafana POST vs. table upsert) could not be captured from telemetry; this fix hardens both paths regardless.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[chcosta]

Recording a deployment annotation

what's the deployment annotation used for? where does it surface?

Narrow the retry predicate to a new IsTransientFailure helper: retry only network errors, timeouts, and 5xx responses (HttpRequestException with StatusCode null or >= 500, or TaskCanceled/OperationCanceled). Non-transient failures now fail fast instead of being retried 10× and rethrown.

this seems good

Wrap the MarkStart and MarkEnd bodies in try/catch that logs the error and returns NoContent(). Best-effort deployment notifications never fail the calling pipeline again.

isn't this just masking the failure? if we're just going to do this, then it sounds like the notifications aren't needed. can we just remove them? was this for rollout scorecard or something else?

[epananth]

There was a transient error where one will have to go delete a file in Azure portal for this service to work. I think Haruna and Meghna did that a few times.

It would be nice if this could cover that case too

[missymessa]

Good questions — and the review consensus (thanks @garath) actually resolves the masking concern by changing the approach:

What the annotation is / where it surfaces: the deployment annotation is a Grafana dashboard annotation — a vertical marker on the time-series dashboards showing when each service was deployed (start→end span, tagged deploy, deploy-<service>, <service>). It's used to correlate metric/behavior changes with deployments while looking at Grafana (not a rollout scorecard).

On "isn't this just masking the failure?": you're right that swallowing a failing Grafana push would just hide a broken notification. So instead of masking it, this revision removes the fragile direct call to the Grafana annotations API entirely. MarkStart/MarkEnd now only write the deployment record to the Azure Table, and Grafana reads the annotations from that table directly. That deletes the HTTP dependency (and its ApiToken + 10× retry-then-rethrow path) that was actually producing the intermittent 500s.

So we keep the useful signal (the durable table record that Grafana renders), and the endpoint no longer has a flaky external call to mask. The best-effort try/catch now only guards the Azure Table write (managed-identity, SDK-retried), so a genuine storage outage still won't fail the deploy pipeline, but there's no longer a Grafana-push failure being hidden. Pushed in b4f7c83.

[missymessa]

Thanks @epananth — that "have to go delete a file in the Azure portal to get the service working again" case is the leftover app_offline.htm, and it's already been fixed separately in #6580 (merged 2026-05-22, WI 8535).

Root cause there: the AzureRmWebAppDeployment@4 task defaults TakeAppOfflineFlag to true, which drops an app_offline.htm during deploy; with zipDeploy that's unnecessary (the deploy is atomic) and intermittently the file wasn't cleaned up, leaving the app offline until someone deleted it manually (what Haruna/Meghna were doing). #6580 sets TakeAppOfflineFlag: false in eng/deploy.yaml so the file is never created.

So that failure mode is covered by #6580; this PR (#6643) is the separate fix for the intermittent 500s from the deployment-notification endpoint. Let me know if you've seen the app_offline.htm situation recur after #6580 shipped — if so I'll reopen that thread.

[missymessa]

There was a transient error where one will have to go delete a file in Azure portal for this service to work. I think Haruna and Meghna did that a few times.

It would be nice if this could cover that case too

@epananth do you still see that happening? I put in a fix for that a while ago: #6580

[garath]

Oh, sorry for swapping the review status. I just noticed that removing grafana-api-token in the config also removes its need to be in the secret-manager manifest. That's a manually-managed secret so removing it is a big win for overhead.

[garath]

Oh, sorry for swapping the review status. I just noticed that removing grafana-api-token in the config also removes its need to be in the secret-manager manifest. That's a manually-managed secret so removing it is a big win for overhead.

These secrets are no longer needed:

• dnceng/.vault-config/dotneteng-status-local.yaml

  Lines 22 to 25 in 3f001e4

  	grafana-api-token:
  	type: azure-managed-grafana-api-key
  	parameters:
  	environment: dotnet-eng-grafana-staging.westus2.cloudapp.azure.com

• dnceng/.vault-config/dotneteng-status-prod.yaml

  Lines 22 to 26 in 3f001e4

  	# Grafana API key with admin privileges
  	grafana-api-token:
  	type: azure-managed-grafana-api-key
  	parameters:
  	environment: dotnet-eng-grafana.westus2.cloudapp.azure.com

• dnceng/.vault-config/dotneteng-status-staging.yaml

  Lines 22 to 26 in 3f001e4

  	# Grafana API key with admin privileges
  	grafana-api-token:
  	type: azure-managed-grafana-api-key
  	parameters:
  	environment: dotnet-eng-grafana-staging.westus2.cloudapp.azure.com

[missymessa]

Done in 189ac85 — removed the grafana-api-token secret from all three vault-config manifests:

• .vault-config/dotneteng-status-local.yaml
• .vault-config/dotneteng-status-prod.yaml
• .vault-config/dotneteng-status-staging.yaml

Confirmed it's safe: that secret was only consumed by the DotNet.Status.Web Grafana-annotations HTTP call (now removed). The dashboard-deploy pipeline (eng/deploy-managed-grafana.yml) uses a separate grafana-admin-api-key in the AMG key vault, and grafana-webhook-secret (AlertHookController) is untouched. Good call on dropping the manually-managed secret.