Skip to content

Commit c1e7924

Browse files
author
Liudmila Molkova
authored
Tracing: Change custom attributes to comply with otel conventions, add tests (Azure#39637)
* Change custom attributes to comply with otel conventions, add tests
1 parent ccc67cd commit c1e7924

File tree

30 files changed

+421
-361
lines changed

30 files changed

+421
-361
lines changed

‎sdk/appconfiguration/Azure.Data.AppConfiguration/CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@
1616

1717
- Added configuration settings snapshot feature which allow users to create a point-in-time snapshot of their configuration store.
1818

19+
### Breaking Changes
20+
21+
- Renamed `key` tag reported on `ConfigurationClient` activities to `az.appconfiguration.key` following OpenTelemetry attribute naming conventions.
22+
23+
### Bugs Fixed
24+
25+
### Other Changes
26+
1927
## 1.3.0-beta.3 (2023-10-09)
2028

2129
### Features Added

‎sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ namespace Azure.Data.AppConfiguration
1616
/// </summary>
1717
public partial class ConfigurationClient
1818
{
19+
private const string OTelAttributeKey = "az.appconfiguration.key";
1920
private readonly SyncTokenPolicy _syncTokenPolicy;
2021

2122
/// <summary>
@@ -179,7 +180,7 @@ public virtual Response<ConfigurationSetting> AddConfigurationSetting(string key
179180
public virtual async Task<Response<ConfigurationSetting>> AddConfigurationSettingAsync(ConfigurationSetting setting, CancellationToken cancellationToken = default)
180181
{
181182
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(AddConfigurationSetting)}");
182-
scope.AddAttribute("key", setting?.Key);
183+
scope.AddAttribute(OTelAttributeKey, setting?.Key);
183184
scope.Start();
184185

185186
try
@@ -219,7 +220,7 @@ public virtual async Task<Response<ConfigurationSetting>> AddConfigurationSettin
219220
public virtual Response<ConfigurationSetting> AddConfigurationSetting(ConfigurationSetting setting, CancellationToken cancellationToken = default)
220221
{
221222
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(AddConfigurationSetting)}");
222-
scope.AddAttribute("key", setting?.Key);
223+
scope.AddAttribute(OTelAttributeKey, setting?.Key);
223224
scope.Start();
224225

225226
try
@@ -291,7 +292,7 @@ public virtual async Task<Response<ConfigurationSetting>> SetConfigurationSettin
291292
{
292293
Argument.AssertNotNull(setting, nameof(setting));
293294
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(SetConfigurationSetting)}");
294-
scope.AddAttribute("key", setting?.Key);
295+
scope.AddAttribute(OTelAttributeKey, setting?.Key);
295296
scope.Start();
296297

297298
try
@@ -333,7 +334,7 @@ public virtual Response<ConfigurationSetting> SetConfigurationSetting(Configurat
333334
{
334335
Argument.AssertNotNull(setting, nameof(setting));
335336
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(SetConfigurationSetting)}");
336-
scope.AddAttribute("key", setting?.Key);
337+
scope.AddAttribute(OTelAttributeKey, setting?.Key);
337338
scope.Start();
338339

339340
try
@@ -425,7 +426,7 @@ public virtual Response DeleteConfigurationSetting(ConfigurationSetting setting,
425426
private async Task<Response> DeleteConfigurationSettingAsync(string key, string label, MatchConditions requestOptions, CancellationToken cancellationToken = default)
426427
{
427428
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(DeleteConfigurationSetting)}");
428-
scope.AddAttribute("key", key);
429+
scope.AddAttribute(OTelAttributeKey, key);
429430
scope.Start();
430431

431432
try
@@ -454,7 +455,7 @@ private async Task<Response> DeleteConfigurationSettingAsync(string key, string
454455
private Response DeleteConfigurationSetting(string key, string label, MatchConditions requestOptions, CancellationToken cancellationToken = default)
455456
{
456457
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(DeleteConfigurationSetting)}");
457-
scope.AddAttribute("key", key);
458+
scope.AddAttribute(OTelAttributeKey, key);
458459
scope.Start();
459460

460461
try
@@ -578,7 +579,7 @@ public virtual Response<ConfigurationSetting> GetConfigurationSetting(Configurat
578579
internal virtual async Task<Response<ConfigurationSetting>> GetConfigurationSettingAsync(string key, string label, DateTimeOffset? acceptDateTime, MatchConditions conditions, CancellationToken cancellationToken = default)
579580
{
580581
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(GetConfigurationSetting)}");
581-
scope.AddAttribute(nameof(key), key);
582+
scope.AddAttribute(OTelAttributeKey, key);
582583
scope.Start();
583584

584585
try
@@ -614,7 +615,7 @@ internal virtual async Task<Response<ConfigurationSetting>> GetConfigurationSett
614615
internal virtual Response<ConfigurationSetting> GetConfigurationSetting(string key, string label, DateTimeOffset? acceptDateTime, MatchConditions conditions, CancellationToken cancellationToken = default)
615616
{
616617
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(GetConfigurationSetting)}");
617-
scope.AddAttribute(nameof(key), key);
618+
scope.AddAttribute(OTelAttributeKey, key);
618619
scope.Start();
619620

620621
try
@@ -1356,7 +1357,7 @@ public virtual Response<ConfigurationSetting> SetReadOnly(ConfigurationSetting s
13561357
private async ValueTask<Response<ConfigurationSetting>> SetReadOnlyAsync(string key, string label, MatchConditions requestOptions, bool isReadOnly, bool async, CancellationToken cancellationToken)
13571358
{
13581359
using DiagnosticScope scope = ClientDiagnostics.CreateScope($"{nameof(ConfigurationClient)}.{nameof(SetReadOnly)}");
1359-
scope.AddAttribute("key", key);
1360+
scope.AddAttribute(OTelAttributeKey, key);
13601361
scope.Start();
13611362

13621363
try

‎sdk/cognitivelanguage/Azure.AI.Language.QuestionAnswering/CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
### Breaking Changes
88

9+
- Renamed `projectName` and `deploymentName` tags reported on `QuestionAnsweringClient` activities to `az.cognitivelanguage.project.name` and `az.cognitivelanguage.deployment.name` following OpenTelemetry attribute naming conventions.
10+
911
### Bugs Fixed
1012

1113
### Other Changes

‎sdk/cognitivelanguage/Azure.AI.Language.QuestionAnswering/src/QuestionAnsweringClient.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ namespace Azure.AI.Language.QuestionAnswering
1616
public class QuestionAnsweringClient
1717
{
1818
internal const string AuthorizationHeader = "Ocp-Apim-Subscription-Key";
19+
private const string OTelProjectNameKey = "az.cognitivelanguage.project.name";
20+
private const string OTelDeploymentNameKey = "az.cognitivelanguage.deployment.name";
1921

2022
private readonly QuestionAnsweringRestClient _restClient;
2123

@@ -136,8 +138,8 @@ private async Task<Response<AnswersResult>> GetAnswersAsync(QuestionAnsweringPro
136138
Argument.AssertNotNull(options, nameof(options));
137139

138140
using DiagnosticScope scope = Diagnostics.CreateScope($"{nameof(QuestionAnsweringClient)}.{nameof(GetAnswers)}");
139-
scope.AddAttribute("projectName", project.ProjectName);
140-
scope.AddAttribute("deploymentName", project.DeploymentName);
141+
scope.AddAttribute(OTelProjectNameKey, project.ProjectName);
142+
scope.AddAttribute(OTelDeploymentNameKey, project.DeploymentName);
141143
scope.Start();
142144

143145
try
@@ -180,8 +182,8 @@ private Response<AnswersResult> GetAnswers(QuestionAnsweringProject project, Ans
180182
Argument.AssertNotNull(options, nameof(options));
181183

182184
using DiagnosticScope scope = Diagnostics.CreateScope($"{nameof(QuestionAnsweringClient)}.{nameof(GetAnswers)}");
183-
scope.AddAttribute("projectName", project.ProjectName);
184-
scope.AddAttribute("deploymentName", project.DeploymentName);
185+
scope.AddAttribute(OTelProjectNameKey, project.ProjectName);
186+
scope.AddAttribute(OTelDeploymentNameKey, project.DeploymentName);
185187
scope.Start();
186188

187189
try

‎sdk/core/Azure.Core.TestFramework/src/Instrumentation/DiagnosticScopeValidatingInterceptor.cs‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics;
77
using System.Linq;
88
using System.Reflection;
9+
using System.Text.RegularExpressions;
910
using System.Threading;
1011
using System.Threading.Tasks;
1112
using Azure.Core.Tests;
@@ -23,6 +24,7 @@ public class DiagnosticScopeValidatingInterceptor : IInterceptor
2324
.GetMethod(nameof(WrapAsyncResultCore), BindingFlags.NonPublic | BindingFlags.Static)
2425
?? throw new InvalidOperationException("Unable to find DiagnosticScopeValidatingInterceptor.WrapAsyncResultCore method");
2526

27+
private static Regex AttributeNameRegex = new Regex(@"^[a-z\._]+$", RegexOptions.Compiled);
2628
public void Intercept(IInvocation invocation)
2729
{
2830
var type = invocation.Method.ReturnType;
@@ -197,6 +199,7 @@ internal static async ValueTask<T> ValidateDiagnosticScope<T>(Func<ValueTask<(T
197199
T result;
198200

199201
using ClientDiagnosticListener diagnosticListener = new ClientDiagnosticListener(s => s.StartsWith("Azure."), asyncLocal: true);
202+
200203
try
201204
{
202205
// activities may be suppressed if they are called in scope of other activities create by other SDK methods.
@@ -220,11 +223,14 @@ internal static async ValueTask<T> ValidateDiagnosticScope<T>(Func<ValueTask<(T
220223
{
221224
// Remove subscribers before enumerating events.
222225
diagnosticListener.Dispose();
226+
223227
var skipOverrideProperty = forwardAttribute is not null ? forwardAttribute.GetType().GetProperty("SkipChecks") : null;
224228
bool skipOverride = skipOverrideProperty is not null ? (bool)skipOverrideProperty.GetValue(forwardAttribute) : false;
225229
skipChecks |= skipOverride;
226230
if (!skipChecks)
227231
{
232+
diagnosticListener.Scopes.ForEach(s => CheckAttributes(s.Activity, strict));
233+
228234
if (strict)
229235
{
230236
ClientDiagnosticListener.ProducedDiagnosticScope e = diagnosticListener.Scopes.FirstOrDefault(e => e.Name == expectedName);
@@ -236,11 +242,6 @@ internal static async ValueTask<T> ValidateDiagnosticScope<T>(Func<ValueTask<(T
236242
$" You may have forgotten to add clientDiagnostics.CreateScope(...), set your operationId to {expectedName} in {source} or applied the Azure.Core.ForwardsClientCallsAttribute to {source}.");
237243
}
238244

239-
if (!e.Activity.Tags.Any(tag => tag.Key == "az.namespace"))
240-
{
241-
throw new InvalidOperationException($"All diagnostic scopes should have 'az.namespace' attribute, make sure the assembly containing **ClientOptions type is marked with the AzureResourceProviderNamespace attribute specifying the appropriate provider. This attribute should be included in AssemblyInfo, and can be included by pulling in AzureResourceProviderNamespaceAttribute.cs using the AzureCoreSharedSources alias.");
242-
}
243-
244245
if (lastException != null && !e.IsFailed)
245246
{
246247
throw new InvalidOperationException($"Expected scope {expectedName} to be marked as failed but it succeeded{Environment.NewLine}Exception: {lastException}");
@@ -264,6 +265,59 @@ internal static async ValueTask<T> ValidateDiagnosticScope<T>(Func<ValueTask<(T
264265
return result;
265266
}
266267

268+
private static void CheckAttributes(Activity activity, bool strict)
269+
{
270+
foreach (var tag in activity.TagObjects)
271+
{
272+
if (tag.Key == "kind" || tag.Key.StartsWith("otel.") || tag.Key == "requestId" || tag.Key == "serviceRequestId")
273+
{
274+
// TODO: these are populated on DiagnosticSource path. We should rewrite this to ActivitySource and remove those.
275+
continue;
276+
}
277+
278+
if (!AttributeNameRegex.IsMatch(tag.Key))
279+
{
280+
throw new InvalidOperationException("Attribute name can only have lowercase letters, dot (`.`), and underscore (`_`). " +
281+
"Use dot to separate namespaces and underscore to separate words (e.g. http.request.status_code). " + $"Attribute name: {tag.Key}");
282+
}
283+
284+
int dot = tag.Key.IndexOf('.');
285+
if (dot == -1)
286+
{
287+
throw new InvalidOperationException("Attribute names must be namespaced. Use OpenTelemetry attributes whenever possible - https://github.com/open-telemetry/semantic-conventions/blob/main/docs/README.md. " +
288+
"Custom Azure-specific attributes must start with `az.` and should have library-specific namespaces (e.g. `az.digital_twin.twin_id`). " + $"Attribute name: {tag.Key}");
289+
}
290+
291+
string ns = tag.Key.Substring(0, dot);
292+
if ("az" != ns && "messaging" != ns && "http" != ns && "error" != ns && "url" != ns)
293+
{
294+
throw new InvalidOperationException("Unknown attribute namespace. Use OpenTelemetry attributes whenever possible - https://github.com/open-telemetry/semantic-conventions/blob/main/docs/README.md." +
295+
"Custom Azure-specific attributes must start with `az.` and should have library-specific namespaces (e.g. `az.digital_twin.twin_id`). " + $"Attribute name: {tag.Key}");
296+
}
297+
298+
string tagValueStr = tag.Value?.ToString();
299+
if (string.IsNullOrEmpty(tagValueStr) || (tag.Key.StartsWith("az.") && tagValueStr.Length > 256))
300+
{
301+
throw new InvalidOperationException("Attribute values must not be null, empty, or longer than 256 characters. "
302+
+ $"Attribute name: `{tag.Key}`, value: `{tag.Value}`, activity name: `{activity.OperationName}`");
303+
}
304+
}
305+
306+
if (strict)
307+
{
308+
if (!activity.Tags.Any(tag => tag.Key == "az.namespace"))
309+
{
310+
throw new InvalidOperationException($"All diagnostic scopes should have 'az.namespace' attribute, make sure the assembly containing **ClientOptions type is marked with the AzureResourceProviderNamespace attribute specifying the appropriate provider. This attribute should be included in AssemblyInfo, and can be included by pulling in AzureResourceProviderNamespaceAttribute.cs using the AzureCoreSharedSources alias. " +
311+
$"\nActivity name: {activity.OperationName}, Source name: {activity?.Source.Name}");
312+
}
313+
}
314+
315+
if (activity.Status == ActivityStatusCode.Error && activity.Source?.HasListeners() == true && !activity.TagObjects.Any(kvp => kvp.Key == "error.type"))
316+
{
317+
throw new InvalidOperationException("All failed activities must have `error.type` attribute set to known or documented error code or a full name of exception type");
318+
}
319+
}
320+
267321
internal class DiagnosticScopeValidatingAsyncEnumerable<T> : AsyncPageable<T>
268322
{
269323
private readonly AsyncPageable<T> _pageable;

‎sdk/core/Azure.Core/src/Pipeline/Internal/RequestActivityPolicy.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,7 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
7878
scope.AddIntegerAttribute("server.port", message.Request.Uri.Port);
7979
}
8080

81-
if (_resourceProviderNamespace != null)
82-
{
83-
scope.AddAttribute("az.namespace", _resourceProviderNamespace);
84-
}
81+
scope.AddAttribute("az.namespace", _resourceProviderNamespace);
8582

8683
if (!isActivitySourceEnabled && message.Request.Headers.TryGetValue("User-Agent", out string? userAgent))
8784
{

0 commit comments

Comments
 (0)