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.

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

Use current activity if there is one #1228

Merged
merged 4 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
15 changes: 11 additions & 4 deletions src/Elastic.Apm.AspNetCore/WebRequestTransactionCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,20 @@ internal static void SetOutcomeForHttpResult(ITransaction transaction, int HttpR
/// <returns>default if it's not a grpc call, otherwise the Grpc method name and result as a tuple </returns>
private static (string methodname, string result) CollectGrpcInfo()
{
var parentActivity = Activity.Current?.Parent;
// gRPC info is stored on the Activity with name `Microsoft.AspNetCore.Hosting.HttpRequestIn`.
// Therefore we follow parent activities as long as we reach this activity.
// Activity.Current can e.g. be the `ElasticApm.Transaction` Activity, so we need to go up the activity chain.
var httpRequestInActivity = Activity.Current;

while (httpRequestInActivity != null && httpRequestInActivity.OperationName != "Microsoft.AspNetCore.Hosting.HttpRequestIn")
httpRequestInActivity = httpRequestInActivity.Parent;

(string methodname, string result) grpcCallInfo = default;

if (parentActivity != null)
if (httpRequestInActivity != null)
{
var grpcMethodName = parentActivity.Tags.FirstOrDefault(n => n.Key == "grpc.method").Value;
var grpcStatusCode = parentActivity.Tags.FirstOrDefault(n => n.Key == "grpc.status_code").Value;
var grpcMethodName = httpRequestInActivity.Tags.FirstOrDefault(n => n.Key == "grpc.method").Value;
var grpcStatusCode = httpRequestInActivity.Tags.FirstOrDefault(n => n.Key == "grpc.status_code").Value;

if (!string.IsNullOrEmpty(grpcMethodName) && !string.IsNullOrEmpty(grpcStatusCode))
grpcCallInfo = (grpcMethodName, grpcStatusCode);
Expand Down
64 changes: 44 additions & 20 deletions src/Elastic.Apm/Model/Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ internal class Transaction : ITransaction
private readonly IApmServerInfo _apmServerInfo;
private readonly Lazy<Context> _context = new Lazy<Context>();
private readonly ICurrentExecutionSegmentsContainer _currentExecutionSegmentsContainer;

private readonly IApmLogger _logger;
private readonly IPayloadSender _sender;

private readonly string _traceState;

// Flags whether the agent created a new Activity for this transaction.
// If there is an existing Activity using W3C format, no Activity will be created and this remains false.
private bool _createdActivity;
russcam marked this conversation as resolved.
Show resolved Hide resolved

// This constructor is meant for serialization
[JsonConstructor]
private Transaction(Context context, string name, string type, double duration, long timestamp, string id, string traceId, string parentId,
Expand Down Expand Up @@ -99,26 +101,44 @@ internal Transaction(
HasCustomName = false;
Type = type;

// For each transaction start, we fire an Activity
// If Activity.Current is null, then we create one with this and set its traceid, which will flow to all child activities and we also reuse it in Elastic APM, so it'll be the same on all Activities and in Elastic
// If Activity.Current is not null, we pick up its traceid and apply it in Elastic APM
// For each transaction start, fire an Activity if we're not ignoring them.
// If Activity.Current is null, then we create one with this and set its traceid, which will flow to all child activities,
// and we also reuse it in Elastic APM, so it'll be the same on all Activities.
// If Activity.Current is not null and using W3C id format, use its traceid and apply it in Elastic APM
if (!ignoreActivity)
StartActivity();
{
var currentActivity = Activity.Current;
if (currentActivity != null && currentActivity.IdFormat == ActivityIdFormat.W3C)
_activity = currentActivity;
else
_activity = StartActivity();
}

var isSamplingFromDistributedTracingData = false;
if (distributedTracingData == null)
{
// Here we ignore Activity.Current.ActivityTraceFlags because it starts out without setting the IsSampled flag, so relying on that would mean a transaction is never sampled.
// Here we ignore Activity.Current.ActivityTraceFlags because it starts out without setting the IsSampled flag,
// so relying on that would mean a transaction is never sampled.
// To be sure activity creation was successful let's check on it
if (_activity != null)
{
// In case activity creation was successful, let's reuse the ids
Id = _activity.SpanId.ToHexString();
// If an activity was created, reuse its id, otherwise generate a new one.
if (_createdActivity)
Id = _activity.SpanId.ToHexString();
else
{
var idBytes = new byte[8];
Id = RandomGenerator.GenerateRandomBytesAsString(idBytes);
}

TraceId = _activity.TraceId.ToHexString();
_traceState = _activity.TraceStateString;

var idBytesFromActivity = new Span<byte>(new byte[16]);
_activity.TraceId.CopyTo(idBytesFromActivity);
// Read right most bits. From W3C TraceContext: "it is important for trace-id to carry "uniqueness" and "randomness" in the right part of the trace-id..."

// Read right most bits. From W3C TraceContext: "it is important for trace-id to carry "uniqueness" and "randomness"
// in the right part of the trace-id..."
idBytesFromActivity = idBytesFromActivity.Slice(8);
IsSampled = sampler.DecideIfToSample(idBytesFromActivity.ToArray());
}
Expand All @@ -133,12 +153,12 @@ internal Transaction(
TraceId = RandomGenerator.GenerateRandomBytesAsString(idBytes);
}

// PrentId could be also set here, but currently in the UI each trace must start with a transaction where the ParentId is null,
// ParentId could be also set here, but currently in the UI each trace must start with a transaction where the ParentId is null,
// so to avoid https://github.com/elastic/apm-agent-dotnet/issues/883 we don't set it yet.
}
else
{
if (_activity != null)
if (_activity != null && _createdActivity)
Id = _activity.SpanId.ToHexString();
else
{
Expand Down Expand Up @@ -177,13 +197,15 @@ internal Transaction(
" Start time: {Time} (as timestamp: {Timestamp})",
this, IsSampled, sampler, TimeUtils.FormatTimestampForLog(Timestamp), Timestamp);
}
}

void StartActivity()
{
_activity = new Activity(ApmTransactionActivityName);
_activity.SetIdFormat(ActivityIdFormat.W3C);
_activity.Start();
}
private Activity StartActivity()
{
var activity = new Activity(ApmTransactionActivityName);
activity.SetIdFormat(ActivityIdFormat.W3C);
activity.Start();
_createdActivity = true;
return activity;
}

/// <summary>
Expand All @@ -192,7 +214,7 @@ void StartActivity()
/// With this, in case Activity.Current is null, the agent will set it and when the next Activity gets created it'll
/// have this activity as its parent and the TraceId will flow to all Activity instances.
/// </summary>
private Activity _activity;
private readonly Activity _activity;

private bool _isEnded;

Expand Down Expand Up @@ -306,6 +328,7 @@ public Outcome Outcome

internal Service Service;


[JsonProperty("span_count")]
public SpanCount SpanCount { get; set; }

Expand Down Expand Up @@ -398,7 +421,8 @@ public void End()
TimeUtils.FormatTimestampForLog(endTimestamp), endTimestamp, Duration);
}

_activity?.Stop();
if (_createdActivity)
_activity?.Stop();

var isFirstEndCall = !_isEnded;
_isEnded = true;
Expand Down