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

Jump to bottom

Disallow setting Parent on the started Activity #81394

Merged
merged 2 commits into from
Jan 31, 2023
Merged

Disallow setting Parent on the started Activity #81394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25900ad
Disallow setting Parent on the started Activity
tarekgh Jan 30, 2023
881314d
Address the feedback
tarekgh Jan 31, 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
3 changes: 3 additions & 0 deletions src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@
<data name="ActivityStartAlreadyStarted" xml:space="preserve">
<value>"Can not start an Activity that was already started"</value>
</data>
<data name="ActivitySetParentAlreadyStarted" xml:space="preserve">
<value>"Can not set parent on already started Activity"</value>
</data>
<data name="EndTimeNotUtc" xml:space="preserve">
<value>"EndTime is not UTC"</value>
</data>
Expand Down
14 changes: 12 additions & 2 deletions src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,12 @@ public Activity SetBaggage(string key, string? value)
/// <param name="parentId">The id of the parent operation.</param>
public Activity SetParentId(string parentId)
{
if (Parent != null)
if (_id != null || _spanId != null)
{
// Cannot set the parent on already started Activity.
NotifyError(new InvalidOperationException(SR.ActivitySetParentAlreadyStarted));
}
else if (Parent != null)
{
NotifyError(new InvalidOperationException(SR.SetParentIdOnActivityWithParent));
}
Expand All @@ -589,7 +594,12 @@ public Activity SetParentId(string parentId)
/// </summary>
public Activity SetParentId(ActivityTraceId traceId, ActivitySpanId spanId, ActivityTraceFlags activityTraceFlags = ActivityTraceFlags.None)
{
if (Parent != null)
if (_id != null || _spanId != null)
{
// Cannot set the parent on already started Activity.
NotifyError(new InvalidOperationException(SR.ActivitySetParentAlreadyStarted));
}
else if (Parent != null)
{
NotifyError(new InvalidOperationException(SR.SetParentIdOnActivityWithParent));
}
Expand Down
22 changes: 19 additions & 3 deletions src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,23 @@ public void Tags()
[Fact]
public void SetParentId()
{
var parent = new Activity("parent");
using (var a = new Activity("foo"))
{
a.Start();
string parentId = a.ParentId;
a.SetParentId("00-6e76af18746bae4eadc3581338bbe8b1-2899ebfdbdce904b-00"); // Error does nothing
Assert.Equal(parentId, a.ParentId);
}

tarekgh marked this conversation as resolved.
Show resolved Hide resolved
using (var a = new Activity("foo"))
{
a.Start();
string parentId = a.ParentId;
a.SetParentId(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom()); // Nothing will happen
Assert.Equal(parentId, a.ParentId);
}

using var parent = new Activity("parent");
parent.SetParentId(null); // Error does nothing
Assert.Null(parent.ParentId);

Expand All @@ -255,7 +271,7 @@ public void SetParentId()
Assert.Equal(parent.ParentId, parent.RootId);
parent.Start();

var child = new Activity("child");
using var child = new Activity("child");
child.Start();

Assert.Equal(parent.Id, child.ParentId);
Expand Down Expand Up @@ -730,7 +746,7 @@ public void IdFormat_W3CInvalidVersionFF()
[Fact]
public void IdFormat_W3CWhenTraceIdAndSpanIdProvided()
{
Activity activity = new Activity("activity3");
using Activity activity = new Activity("activity3");
ActivityTraceId activityTraceId = ActivityTraceId.CreateRandom();
activity.SetParentId(activityTraceId, ActivitySpanId.CreateRandom());
activity.Start();
Expand Down