Skip to content

Fix StartOperation/StopOperation losing or misreporting telemetry fields - #3225

Merged
Rajkumar Rangaraj (rajkumar-rangaraj) merged 8 commits into
mainfrom
rajrang/fixStartOperationTags
Oct 8, 2026
Merged

Rajkumar Rangaraj (rajkumar-rangaraj) merged 8 commits into
mainfrom
rajrang/fixStartOperationTags

Conversation

@rajkumar-rangaraj

@rajkumar-rangaraj Rajkumar Rangaraj (rajkumar-rangaraj) commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Fix Issue # (no existing issue; found while investigating #3223).

Changes

StartOperation/StopOperation lost or misreported fields set on the operation telemetry. On dispose, OperationHolder only mapped DependencyTelemetry, through ActivityShimMapper, which wrote attribute keys the Azure Monitor exporter doesn't use the way it intended. RequestTelemetry fields and custom Properties were never applied.

Exported envelopes from main, compared with TrackDependency/TrackRequest for the same values:

Scenario Expected (what Track* gives) StartOperation on main
Http dependency data = URL, resultCode = 200 data = null, resultCode = 0
SQL dependency resultCode = 0 null
Queue Message dependency type Queue Message, data myqueue type queue, data = the host, target = the full URL, queue name in a messaging.destination custom dimension
Other dependency types data set null
Request url, responseCode, success as set url null, responseCode 0, Success = false ignored
Telemetry.Properties exported dropped (requests and dependencies)
Every operation none stray OperationName custom dimension

Root causes:

  • The shim set the old-convention http.method together with new-convention keys (url.full, http.response.status_code). The exporter picks its old-convention HTTP path based on http.method and then reads http.url/http.status_code, so it found neither.
  • It wrote messaging.destination instead of messaging.destination.name, and wrote the result code for every dependency type as http.response.status_code.
  • StartOperation added an OperationName tag to the activity. It carried state between operations in 2.x, but nothing reads it in 3.x and the exporter emits it as a custom dimension.

Fix (small, contained in the shim):

  • ActivityShimMapper now writes the same microsoft.dependency.* / microsoft.request.* override keys that TrackDependency/TrackRequest already use, and only for values that are set.
  • It now maps RequestTelemetry (Url, Source, ResponseCode) as well as DependencyTelemetry.
  • It copies custom Properties whose keys aren't already on the activity, so tag values seeded into Properties by StartOperation(Activity) can't roll back later instrumentation updates. It sets the status only when Success has a value. A request that never sets Success therefore isn't reported as failed; DependencyTelemetry defaults to Success = true, as before.
  • OperationHolder.Dispose calls the request mapping alongside the existing dependency mapping.
  • StartOperation no longer adds the OperationName tag, and ActivityExtensions.SetOperationName is removed.
  • Because only values that are set are applied, an instrumented activity passed to StartOperation(Activity) keeps the fields the exporter derives from it unless the caller sets them.

Out of scope: item-level Context and operation name. StartOperation still doesn't export an operation name, as on main.

Tests

  • Added StartOperationExportTests. It runs StartOperation through the real exporter with a capturing HttpPipelineTransport and asserts on the exported envelopes. It covers dependency fields for Http, SQL, Queue Message and custom types; the StartOperation(T) overload; request url, source, responseCode and success; properties; the StartOperation(Activity) overload with explicitly set fields, with instrumentation updates made after start, and with an untouched instrumented activity; and that no unmapped keys appear in customDimensions. Most fail against main. The two caller-activity guards (the untouched activity and instrumentation updates after start) are expected to pass on main too.
    • It runs in its own collection with parallelization disabled. A tracer provider from a test class running at the same time makes both Azure Monitor samplers add microsoft.sample_rate to the same activity, which throws.
  • Updated the TelemetryClientExtensionTests that asserted on the removed OperationName activity tag.
  • BASE tests pass locally on net8.0 and net462, except FeatureMetricEmissionHelperTests.ReportsFeaturesSeen. That test also fails on clean main in this environment (it expects unknown and gets vm) and is unrelated. TelemetryClientContextTagsTest.StartOperation_GlobalPropertiesApplied fails intermittently in full-suite runs with a duplicate microsoft.sample_rate exception thrown from ActivitySource.StartActivity, which happens when another test class's tracer provider is active at the same time. It reproduces on pure main (without this PR's tests) and is unrelated.

Checklist

  • I ran Unit Tests locally.
  • CHANGELOG.md updated with one line description of the fix, and a link to the original issue if available.

For significant contributions please make sure you have completed the following items:

  • Design discussion issue #
  • Changes in public surface reviewed (no public API changes)

OperationHolder.Dispose only mapped DependencyTelemetry, via ActivityShimMapper, which
wrote keys the Azure Monitor exporter does not use as intended:
- HTTP: http.method (old convention) mixed with url.full/http.response.status_code
  (new convention), so Data was empty and ResultCode was 0.
- SQL/custom types: ResultCode and Data were dropped.
- Queue Message: Type became 'queue', Data became the host, and the queue name was
  exported as a messaging.destination custom dimension.
RequestTelemetry fields (Url, ResponseCode, Success) and custom Properties were never
applied, and StartOperation added an 'OperationName' tag that was exported as a custom
dimension on every operation.

- Extract the TrackDependency/TrackRequest activity mapping (Microsoft override attributes,
  global/custom properties, item-level context) into shared internal helpers and use them
  from OperationHolder.Dispose for SDK-created activities.
- Only export a dependency's operation name when it was explicitly assigned, so a child
  dependency's own name is not reported as its operation name.
- Leave the activity status untouched when Success is not set.
- Remove ActivityShimMapper and the OperationName tag.
- Add tests that run StartOperation through the Azure Monitor exporter and assert on the
  exported envelopes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 21:31
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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.

Copilot review overview

🟡 Changes recommended

The telemetry-object overload still discards an explicitly pre-populated dependency operation name.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Aligns StartOperation/StopOperation exports with TrackRequest and TrackDependency.

Changes:

  • Reuses shared telemetry-to-activity mapping.
  • Removes obsolete shim tags and mapper.
  • Adds exporter-level regression tests.
File Description
CHANGELOG.md Documents the fix.
TelemetryClientExtensionTests.cs Updates operation-name assertions.
StartOperationExportTests.cs Tests exported envelopes.
TelemetryClientExtensions.cs Enables dispose-time mapping.
TelemetryClient.cs Extracts shared mapping helpers.
ActivityShimMapper.cs Removes obsolete mapping.
OperationHolder.cs Applies telemetry before stopping activities.
ActivityExtensions.cs Removes the unmapped operation-name tag helper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread BASE/src/Microsoft.ApplicationInsights/TelemetryClientExtensions.cs Outdated
Review feedback: the previous commit stopped applying dependency fields to caller-owned
activities (StartOperation<T>(Activity)), whereas the old ActivityShimMapper applied them,
so e.g. Success = false on a wrapped dependency was lost.

For caller-owned activities, Dispose now applies only fields the caller explicitly set:
the dependency/request override attributes that have a value, the name only if it was
changed, and the status when Success has a value. Properties and context are not copied,
so the caller's own instrumentation is not overwritten.

Also give the export tests unique operation names and add caller-activity tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot review feedback: StartOperation(T) overwrote a caller-provided
Context.Operation.Name with the telemetry's own Name, and OperationHolder then treated
that value as the default and never exported it.

StartOperation(T) now only defaults Context.Operation.Name when it is empty (matching
2.x), and passes the SDK-generated default to OperationHolder explicitly, so a
caller-provided operation name is exported as ai.operation.name.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cy/TrackRequest

Revert the TelemetryClient refactor and the OperationHolder ownership/operation-name
tracking. Instead, ActivityShimMapper (called from OperationHolder.Dispose) now:
- writes the microsoft.dependency.* / microsoft.request.* override keys that
  TrackDependency/TrackRequest already use, only for values that are set;
- maps RequestTelemetry (Url, Source, ResponseCode) as well as DependencyTelemetry;
- copies custom Properties, syncs a renamed Name to the activity display name, and sets
  the status only when Success has a value.
StartOperation no longer adds the unmapped OperationName tag.

Item-level Context and operation name are intentionally left out of scope; StartOperation
keeps not exporting an operation name, as on main.

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

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.

Copilot review overview

🟡 Changes recommended

Disposal can overwrite activity names and tags updated by instrumentation after StartOperation(Activity).

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
Resolved since last review (1)

Comment thread BASE/src/Microsoft.ApplicationInsights/Internal/ActivityShimMapper.cs Outdated
Copilot review feedback:
- StartOperation(Activity) seeds Telemetry.Name and Properties from the activity, so
  re-applying them on dispose could undo later instrumentation updates. Drop the Name ->
  DisplayName sync, and only set Properties whose keys are not already on the activity
  (client GlobalProperties are applied in OnEnd only when absent, so item properties still
  take precedence).
- Assert RequestTelemetry.Source in the export test.

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

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.

Copilot review overview

🟡 Changes recommended

Property precedence can silently discard explicit edits to properties seeded from caller activities.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

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.

Copilot review overview

🟢 Approval recommended

The contained mapping changes are consistent with existing Track APIs and are covered by focused end-to-end tests.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Resolves the CHANGELOG.md conflict by keeping both Unreleased entries (#3224 from main and #3225 from this branch) in ascending PR-number order.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Oct 19, 2026
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