Skip to content
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

OTel: Fix duplicated attributes and suppression #40027

Merged
merged 2 commits into from
Nov 15, 2023
Merged

OTel: Fix duplicated attributes and suppression #40027

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5963fa9
Fix duplicated attributes and suppression
lmolkova Nov 15, 2023
2f50557
docs corrections
lmolkova Nov 15, 2023
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
13 changes: 3 additions & 10 deletions sdk/core/Azure.Core/src/Pipeline/Internal/RequestActivityPolicy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,13 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip

if (message.Response.IsError)
{
if (isActivitySourceEnabled)
{
scope.AddAttribute("error.type", statusCodeStr);
}
else
{
scope.AddAttribute("otel.status_code", "ERROR");
}
scope.Failed(statusCodeStr);
}
else if (!isActivitySourceEnabled)

if (!isActivitySourceEnabled)
{
// Set the status to UNSET so the AppInsights doesn't try to infer it from the status code
scope.AddAttribute("otel.status_code", "UNSET");
scope.AddAttribute("otel.status_code", message.Response.IsError ? "ERROR" : "UNSET");
}
}

Expand Down
9 changes: 6 additions & 3 deletions sdk/core/Azure.Core/src/Shared/DiagnosticScope.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ public void AddLink(string traceparent, string? tracestate, IDictionary<string,
public void Start()
{
Activity? started = _activityAdapter?.Start();
started?.SetCustomProperty(AzureSdkScopeLabel, AzureSdkScopeValue);
if (_suppressNestedClientActivities)
{
started?.SetCustomProperty(AzureSdkScopeLabel, AzureSdkScopeValue);
}
}

public void SetDisplayName(string displayName)
Expand Down Expand Up @@ -261,7 +264,7 @@ public void AddTag(string name, object value)
_tagCollection?.Add(new KeyValuePair<string, object>(name, value!));
#else
_tagCollection ??= new ActivityTagsCollection();
_tagCollection.Add(name, value!);
_tagCollection[name] = value!;
lmolkova marked this conversation as resolved.
Show resolved Hide resolved
#endif
}
else
Expand Down Expand Up @@ -550,7 +553,7 @@ private void AddObjectTag(string name, object value)
#else
if (_activitySource?.HasListeners() == true)
{
_currentActivity?.AddTag(name, value);
_currentActivity?.SetTag(name, value);
}
else
{
Expand Down
46 changes: 37 additions & 9 deletions sdk/core/Azure.Core/tests/ClientDiagnosticsTests.Net50.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,34 @@ public void StableStartsActivityShortNameSourceActivity()
CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>(DiagnosticScope.OpenTelemetrySchemaAttribute, DiagnosticScope.OpenTelemetrySchemaVersion));
}

[Test]
[NonParallelizable]
public void DuplicateTagsAreUpdatedActivitySource()
{
using var activityListener = new TestActivitySourceListener("Azure.Clients.ActivityName");
DiagnosticScopeFactory clientDiagnostics = new DiagnosticScopeFactory("Azure.Clients", "Microsoft.Azure.Core.Cool.Tests", true, false, true);

DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
scope.AddAttribute("key1", "value1");
scope.AddAttribute("key1", "value2");

scope.Start();
scope.AddAttribute("key2", "value1");
scope.AddAttribute("key2", "value2");

scope.Dispose();

Assert.AreEqual(1, activityListener.Activities.Count);
var activity = activityListener.Activities.Dequeue();

var key1 = activity.TagObjects.Where(kvp => kvp.Key == "key1").ToList();
var key2 = activity.TagObjects.Where(kvp => kvp.Key == "key2").ToList();
Assert.AreEqual(1, key1.Count);
Assert.AreEqual(1, key2.Count);
Assert.AreEqual("value2", key1.Single().Value);
Assert.AreEqual("value2", key2.Single().Value);
}

[TestCase(null)]
[TestCase(false)]
[TestCase(true)]
Expand Down Expand Up @@ -176,7 +204,7 @@ public void NestedClientActivitiesSuppressed(int kind, bool expectSuppression)
using DiagnosticScope scope = clientDiagnostics.CreateScope("ClientName.ActivityName", (ActivityKind)kind);
scope.Start();

DiagnosticScope nestedScope = clientDiagnostics.CreateScope("ClientName.NestedActivityName", (ActivityKind)kind);
DiagnosticScope nestedScope = clientDiagnostics.CreateScope("ClientName.NestedActivityName", ActivityKind.Client);
nestedScope.Start();
if (expectSuppression)
{
Expand All @@ -199,7 +227,7 @@ public void NestedClientActivitiesSuppressed(int kind, bool expectSuppression)
[TestCase(ActivityKind.Producer, false)]
[TestCase(ActivityKind.Consumer, false)]
[NonParallelizable]
public void NestedClientActivitiesSuppressionActivitySource(int kind, bool expectSuppression)
public void NestedInternalActivitiesSuppressionActivitySource(int kind, bool expectSuppression)
{
using var activityListener = new TestActivitySourceListener("Azure.Clients.ClientName");

Expand All @@ -208,7 +236,7 @@ public void NestedClientActivitiesSuppressionActivitySource(int kind, bool expec
using DiagnosticScope scope = clientDiagnostics.CreateScope("ClientName.ActivityName", (ActivityKind)kind);
scope.Start();

DiagnosticScope nestedScope = clientDiagnostics.CreateScope("ClientName.NestedActivityName", (ActivityKind)kind);
DiagnosticScope nestedScope = clientDiagnostics.CreateScope("ClientName.NestedActivityName", ActivityKind.Internal);
nestedScope.Start();
if (expectSuppression)
{
Expand All @@ -224,12 +252,12 @@ public void NestedClientActivitiesSuppressionActivitySource(int kind, bool expec
Assert.AreEqual("ClientName.ActivityName", Activity.Current.OperationName);
}

[TestCase(true, true, true)]
[TestCase(true, false, false)]
[TestCase(false, true, true)]
[TestCase(false, false, false)]
[TestCase(true, true)]
[TestCase(true, false)]
[TestCase(false, true)]
[TestCase(false, false)]
[NonParallelizable]
public void NestedActivitiesSuppressionConfiguration(bool suppressOuter, bool suppressNested, bool expectSuppression)
public void NestedActivitiesSuppressionConfiguration(bool suppressOuter, bool suppressNested)
{
using var activityListener = new TestActivitySourceListener("Azure.Clients.ClientName");
DiagnosticScopeFactory clientDiagnostics = new DiagnosticScopeFactory("Azure.Clients", "Microsoft.Azure.Core.Cool.Tests", true, suppressOuter, true);
Expand All @@ -241,7 +269,7 @@ public void NestedActivitiesSuppressionConfiguration(bool suppressOuter, bool su
DiagnosticScope nestedScope = clientDiagnostics2.CreateScope("ClientName.NestedActivityName");
nestedScope.Start();

if (expectSuppression)
if (suppressOuter && suppressNested)
{
Assert.IsFalse(nestedScope.IsEnabled);
Assert.AreEqual("ClientName.ActivityName", Activity.Current.OperationName);
Expand Down
Loading