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
29 changes: 29 additions & 0 deletions docs/Datadog.Trace/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,35 @@ Note: Call-target instrumentation does not support instrumenting custom implemen

The `integrations.json` file is no longer required for instrumentation. You can remove references to this file, for example by deleting the `DD_INTEGRATIONS` environment variable.

### User Identification

The tracer provides a convenient method to link an actor to a trace. You have to pass a `UserDetails` object with at least its Id property set to a non-null value.

To correlate users to web requests, you would need to use the following code snippet within each web request, after user authorization has been performed:

```csharp
using Datadog.Trace;

// ...

var userDetails = new UserDetails()
{
// the systems internal identifier for the users
Id = "d41452f2-483d-4082-8728-171a3570e930",
// the email address of the user
Email = "[email protected]",
// the user's name, as displayed by the system
Name = "Jane Doh",
// the user's session id
SessionId = "d0632156-132b-4baa-95b2-a492c5f9cb16",
// the role the user is making the request under
Role = "standard",
};
Tracer.Instance.ActiveScope?.Span.SetUser(userDetails);

```

## Get in touch

If you have questions, feedback, or feature requests, reach our [support](https://docs.datadoghq.com/help).

69 changes: 69 additions & 0 deletions tracer/src/Datadog.Trace/SpanExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// <copyright file="SpanExtensions.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using Datadog.Trace.Util;

namespace Datadog.Trace
{
/// <summary>
/// Extension methods for the <see cref="ISpan"/> interface
/// </summary>
public static class SpanExtensions
{
/// <summary>
/// Sets the details of the user on the local root span
/// </summary>
/// <param name="span">The span to be tagged</param>
/// <param name="userDetails">The details of the current logged on user</param>
public static void SetUser(this ISpan span, UserDetails userDetails)
Comment thread
robertpi marked this conversation as resolved.
{
if (span is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(span));
}

if (userDetails is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(userDetails));
}

if (userDetails.Id is null)
{
ThrowHelper.ThrowArgumentException(nameof(userDetails) + ".Id must be set to a value other than null", nameof(userDetails));
}

var localRootSpan = span;
if (span is Span spanClass)
{
localRootSpan = spanClass.Context.TraceContext?.RootSpan ?? span;
}

if (userDetails.Id is not null)
{
localRootSpan.SetTag(Tags.User.Id, userDetails.Id);
}

if (userDetails.Email is not null)
{
localRootSpan.SetTag(Tags.User.Email, userDetails.Email);
}

if (userDetails.Name is not null)
{
localRootSpan.SetTag(Tags.User.Name, userDetails.Name);
}

if (userDetails.SessionId is not null)
{
localRootSpan.SetTag(Tags.User.SessionId, userDetails.SessionId);
}

if (userDetails.Role is not null)
{
localRootSpan.SetTag(Tags.User.Role, userDetails.Role);
}
}
}
}
9 changes: 9 additions & 0 deletions tracer/src/Datadog.Trace/Tags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -461,5 +461,14 @@ public static class Tags
internal const string GrpcMethodService = "grpc.method.service";
internal const string GrpcMethodName = "grpc.method.name";
internal const string GrpcStatusCode = "grpc.status.code";

internal static class User
{
internal const string Email = "usr.email";
internal const string Name = "usr.name";
internal const string Id = "usr.id";
internal const string SessionId = "usr.session_id";
internal const string Role = "usr.role";
}
}
}
38 changes: 38 additions & 0 deletions tracer/src/Datadog.Trace/UserDetails.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// <copyright file="UserDetails.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

namespace Datadog.Trace
{
/// <summary>
/// A data container class for the users details
/// </summary>
public class UserDetails

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If users are expected to create UserDetails on every request/trace, we should consider making this a struct (maybe an immutable readonly struct?) to avoid the heap allocation. Since this would not be a tiny struct, we could also make the UserDetails parameter in the extension method an in parameter, so it's passed by reference.

If users decide to cache the UserDetails in the user session, it will end up in the heap, but that's still only one instance per user instead of one per request.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with the struct part, but I don't think we should use in when it's not a readonly struct.

From the docs:

Pass a struct as the argument for an in parameter only if it's declared with the readonly modifier or the method accesses only readonly members of the struct. Otherwise, the compiler must create defensive copies in many situations to ensure that arguments are not mutated. Consider the following example that calculates the distance of a 3D point from the origin:

Without some language guarantee, the compiler must create a temporary copy of the argument before calling any member not marked with the readonly modifier. The temporary storage is created on the stack, the values of the argument are copied to the temporary storage, and the value is copied to the stack for each member access as the this argument. In many situations, these copies harm performance enough that pass-by-value is faster than pass-by-read-only-reference when the argument type isn't a readonly struct and the method calls members that aren't marked readonly.

@tonyredondo tonyredondo Apr 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the idea is to use readonly struct and have best of both worlds, an struct that can be cached if the user wants (allocated in the heap) or avoid the heap allocation and do stack allocation if they want to use new in each request.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I mentioned readonly struct but I wasn't sure if we should enforce it. Thank you both for keeping me honest 🎉

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The trouble with readonly struct here is that it makes the API worse IMO. No better than just having parameters passed to the method. Either way would be a breaking change if we want to add an extra parameter.

Struct is still an improvement, we just have to accept the copy I think 🤷‍♂️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No better than just having parameters passed to the method.

I forgot this is how UserDetails came to be in the first place 🤦🏽‍♂️

{
/// <summary>
/// Gets or sets the user's email address
/// </summary>
public string Email { get; set; }

/// <summary>
/// Gets or sets the user's name as displayed in the UI
/// </summary>
public string Name { get; set; }

/// <summary>
/// Gets or sets the unique identifier assoicated with the users
/// </summary>
public string Id { get; set; }

/// <summary>
/// Gets or sets the user's session unique identifier
/// </summary>
public string SessionId { get; set; }

/// <summary>
/// Gets or sets the role associated with the user
/// </summary>
public string Role { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ namespace Datadog.Trace
public Datadog.Trace.ISpanContext Parent { get; set; }
public System.DateTimeOffset? StartTime { get; set; }
}
public static class SpanExtensions
{
public static void SetUser(this Datadog.Trace.ISpan span, Datadog.Trace.UserDetails userDetails) { }
}
public static class SpanKinds
{
public const string Client = "client";
Expand Down Expand Up @@ -315,6 +319,15 @@ namespace Datadog.Trace
public Datadog.Trace.IScope StartActive(string operationName, Datadog.Trace.SpanCreationSettings settings) { }
public static void Configure(Datadog.Trace.Configuration.TracerSettings settings) { }
}
public class UserDetails
{
public UserDetails() { }
public string Email { get; set; }
public string Id { get; set; }
public string Name { get; set; }
public string Role { get; set; }
public string SessionId { get; set; }
}
}
namespace Datadog.Trace.ExtensionMethods
{
Expand Down
171 changes: 171 additions & 0 deletions tracer/test/Datadog.Trace.Tests/TracerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// </copyright>

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Threading.Tasks;
Expand Down Expand Up @@ -524,5 +525,175 @@ public async Task ForceFlush()

agent.Verify(a => a.FlushTracesAsync(), Times.Once);
}

[Fact]
public void SetUserOnRootSpanDirectly()
Comment thread
robertpi marked this conversation as resolved.
{
var scopeManager = new AsyncLocalScopeManager();

var settings = new TracerSettings
{
StartupDiagnosticLogEnabled = false
};
var tracer = new Tracer(settings, Mock.Of<IAgentWriter>(), Mock.Of<ISampler>(), scopeManager, Mock.Of<IDogStatsd>());

var rootTestScope = tracer.StartActive("test.trace");
var childTestScope = tracer.StartActive("test.trace.child");
childTestScope.Dispose();

var email = "[email protected]";
var name = "Jane Doh";
var id = Guid.NewGuid().ToString();
var sessionId = Guid.NewGuid().ToString();
var role = "admin";

var userDetails = new UserDetails()
{
Email = email,
Name = name,
Id = id,
SessionId = sessionId,
Role = role,
};
tracer.ActiveScope?.Span.SetUser(userDetails);

Assert.Equal(email, rootTestScope.Span.GetTag(Tags.User.Email));
Assert.Equal(name, rootTestScope.Span.GetTag(Tags.User.Name));
Assert.Equal(id, rootTestScope.Span.GetTag(Tags.User.Id));
Assert.Equal(sessionId, rootTestScope.Span.GetTag(Tags.User.SessionId));
Assert.Equal(role, rootTestScope.Span.GetTag(Tags.User.Role));
}

[Fact]
public void SetUserOnChildChildSpan_ShouldAttachToRoot()
{
var scopeManager = new AsyncLocalScopeManager();

var settings = new TracerSettings
{
StartupDiagnosticLogEnabled = false
};
var tracer = new Tracer(settings, Mock.Of<IAgentWriter>(), Mock.Of<ISampler>(), scopeManager, Mock.Of<IDogStatsd>());

var rootTestScope = tracer.StartActive("test.trace");
var childTestScope = tracer.StartActive("test.trace.child");

var email = "[email protected]";
var name = "Jane Doh";
var id = Guid.NewGuid().ToString();
var sessionId = Guid.NewGuid().ToString();
var role = "admin";

var userDetails = new UserDetails()
{
Email = email,
Name = name,
Id = id,
SessionId = sessionId,
Role = role,
};
tracer.ActiveScope?.Span.SetUser(userDetails);

childTestScope.Dispose();

Assert.Equal(email, rootTestScope.Span.GetTag(Tags.User.Email));
Assert.Equal(name, rootTestScope.Span.GetTag(Tags.User.Name));
Assert.Equal(id, rootTestScope.Span.GetTag(Tags.User.Id));
Assert.Equal(sessionId, rootTestScope.Span.GetTag(Tags.User.SessionId));
Assert.Equal(role, rootTestScope.Span.GetTag(Tags.User.Role));
}

[Fact]
public void SetUser_ShouldWorkOnAnythingImplementingISpan()
{
var testSpan = new SpanStub();

var email = "[email protected]";
var name = "Jane Doh";
var id = Guid.NewGuid().ToString();
var sessionId = Guid.NewGuid().ToString();
var role = "admin";

var userDetails = new UserDetails()
{
Email = email,
Name = name,
Id = id,
SessionId = sessionId,
Role = role,
};
testSpan.SetUser(userDetails);

Assert.Equal(email, testSpan.GetTag(Tags.User.Email));
Assert.Equal(name, testSpan.GetTag(Tags.User.Name));
Assert.Equal(id, testSpan.GetTag(Tags.User.Id));
Assert.Equal(sessionId, testSpan.GetTag(Tags.User.SessionId));
Assert.Equal(role, testSpan.GetTag(Tags.User.Role));
}

[Fact]
public void SetUser_ShouldThrowAnExceptionIfNoIdIsProvided()
{
var testSpan = new SpanStub();

var email = "[email protected]";

var userDetails = new UserDetails()
{
Email = email,
};

Assert.ThrowsAny<ArgumentException>(() =>
testSpan.SetUser(userDetails));
}

private class SpanStub : ISpan
{
private Dictionary<string, string> _tags = new Dictionary<string, string>();

public string OperationName { get; set; }

public string ResourceName { get; set; }

public string Type { get; set; }

public bool Error { get; set; }

public string ServiceName { get; set; }

public ulong TraceId => 1ul;

public ulong SpanId => 1ul;

public ISpanContext Context => null;

public void Dispose()
{
}

public void Finish()
{
}

public void Finish(DateTimeOffset finishTimestamp)
{
}

public string GetTag(string key)
{
_tags.TryGetValue(key, out var value);
return value;
}

public void SetException(Exception exception)
{
}

public ISpan SetTag(string key, string value)
{
_tags[key] = value;
return this;
}
}
}
}