Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions src/Temporalio.Extensions.OpenTelemetry/TracingInterceptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,21 @@ protected virtual IDictionary<string, string> HeadersFromContext(
return ret;
}

/// <summary>
/// Create tag collection for the given standalone activity ID.
/// </summary>
/// <param name="activityId">Standalone activity ID.</param>
/// <returns>Tags.</returns>
protected virtual IEnumerable<KeyValuePair<string, object?>> CreateStandaloneActivityTags(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could just inline this logic now that start_activity is the only event we are adding tracing to.

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.

I think it's fine to leave here as-is. There's precedent for this pattern.

string activityId)
{
if (Options.TagNameActivityId is string name)
{
return new KeyValuePair<string, object?>[] { new(name, activityId) };
}
return Enumerable.Empty<KeyValuePair<string, object?>>();
}

private static void RecordExceptionWithStatus(Activity? activity, Exception exception)
{
// If the exception is a benign exception, we do not consider the status an error. Note,
Expand Down Expand Up @@ -394,6 +409,31 @@ public override async Task<WorkflowUpdateHandle<TResult>> StartWorkflowUpdateAsy
}
}

public override async Task<ActivityHandle<TResult>> StartActivityAsync<TResult>(
StartActivityInput input)
{
using (var activity = ClientSource.StartActivity(
$"StartActivity:{input.Activity}",
kind: ActivityKind.Client,
parentContext: default,
tags: input.Options.Id is string id ? root.CreateStandaloneActivityTags(id) : null))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input.Options.Id is optional because it uses the shared StartActivityInput. AFAICT, it will never be null for standalone activities.

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.

It shouldn't be null (and is documented that it is required), but customers can still pass null. Given the current type declaration, you are doing the right thing here. The validation of the value is done within the innermost interceptor, which hasn't been called at this point.

{
if (HeadersFromContext(input.Headers) is Dictionary<string, Payload> headers)
{
input = input with { Headers = headers };
}
try
{
return await base.StartActivityAsync<TResult>(input).ConfigureAwait(false);
}
catch (Exception e)
{
RecordExceptionWithStatus(activity, e);
throw;
}
}
}

/// <summary>
/// Serialize current context to headers if one exists.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,27 @@ public async Task TracingInterceptor_Nexus_HasTracing()
"RunWorkflow:TracingWorkflow"));
}

[Fact]
public async Task TracingInterceptor_StandaloneActivity_HasProperSpans()
{
var activityId = $"act-{Guid.NewGuid()}";
var spans = await WithTracingWorkerAsync(async (client, _) =>
{
await client.StartActivityAsync(
"TestStandaloneActivitySpans",
Array.Empty<object?>(),
new(activityId, $"standalone-tq-{Guid.NewGuid()}")
{
ScheduleToCloseTimeout = TimeSpan.FromMinutes(5),
});
});

var activityTags = new[] { ActivityAssertion.TagEqual("temporalActivityID", activityId) };
AssertActivities(
spans,
new ActivityAssertion("StartActivity:TestStandaloneActivitySpans", Parent: null, Tags: activityTags));
}

private static void AssertActivities(
IReadOnlyCollection<Activity> activities, params ActivityAssertion[] assertions)
{
Expand Down Expand Up @@ -886,7 +907,7 @@ private async Task<IReadOnlyCollection<Activity>> WithTracingWorkerAsync(
var activities = new List<Activity>();

// Setup provider
using var tracerProvider = global::OpenTelemetry.Sdk.CreateTracerProviderBuilder().
using var _ = global::OpenTelemetry.Sdk.CreateTracerProviderBuilder().
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a second to realize that tracerProvider was not actually being used we were just using using for lifetime management. Swapped to discard variable instead which preserves lifetime management but better signals intent.

AddSource(
TracingInterceptor.ClientSource.Name,
TracingInterceptor.WorkflowsSource.Name,
Expand Down
Loading