-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Implement OpenTelemetry infrastructure #11255
Open
JanProvaznik
wants to merge
59
commits into
dotnet:main
Choose a base branch
from
JanProvaznik:otel-basic
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
ff9341d
implement, add tests
JanProvaznik 9fe19f8
Merge branch 'main' into otel-basic
JanProvaznik a2f0720
update app.config s
JanProvaznik 96eeeee
Merge branch 'otel-basic' of https://github.com/JanProvaznik/msbuild …
JanProvaznik 49467bb
fix test envvar behavior
JanProvaznik cdaa52a
apply feedback
JanProvaznik 2c28a87
don't use ConversionUtilities
JanProvaznik cf089e9
fix: dispose before flushing
JanProvaznik 7b88424
Move telemetry envvar handling to Traits
JanProvaznik 49c429d
downgrade packages, manage lifetime of telemetry in BuildManager
JanProvaznik c95245c
test what happens in ci with otel
JanProvaznik 9189121
testing in ci
JanProvaznik 9dfb8c9
remove explicit adding source to exporter, wildcard from VS default h…
JanProvaznik 3385704
remove assembly redirect
JanProvaznik a43acbd
remove binding redirect
JanProvaznik b2f0601
remove explicit Extensions primitives reference
JanProvaznik fab67b5
undo binding asyncinterface binding redirect deletion
JanProvaznik da883a9
reintroduce diagnosticsource redirect
JanProvaznik 1c6a099
nit
JanProvaznik de12d05
reintroduce extensions binding redirect...
JanProvaznik a208e67
downgrade diagnosticsource, upgrade VSOTel to VS compatible verison
JanProvaznik eba5787
downgrade diagnosticsource, privateassets vs otel
JanProvaznik f05135f
remove Extensions primitives redirect
JanProvaznik 6ef7496
Revert "remove Extensions primitives redirect"
JanProvaznik 06ef018
Merge branch 'main' of https://github.com/dotnet/msbuild into otel-basic
JanProvaznik 9ceb237
Avoid ValueTuple build error
rainersigwald 5fee7a5
Merge branch 'otel-basic' of https://github.com/JanProvaznik/msbuild …
JanProvaznik e0f9312
reference correct diagnosticsource in version.details.xml
JanProvaznik 735f6e2
include otel in test packages
JanProvaznik db283b0
mention places that need to upgrade vs version on major release
JanProvaznik 91afbfa
17.14 changewave = optout
JanProvaznik 41a7879
fix logic in GetProperties
JanProvaznik 8c0c825
remove explicit references that are transitively included
JanProvaznik 91643e3
remove vs-impl package source
JanProvaznik 802dfc0
adjust VS package
JanProvaznik bb72029
move telemetrystate inside the class it's relevant to
JanProvaznik a1ffd51
Revert "remove vs-impl package source"
JanProvaznik f4befb6
Merge branch 'main' into otel-basic
JanProvaznik 559f85d
add Otel to dependent assemblies
JanProvaznik 17e7628
add all transitive dependencies to app.amd64.config and ngen
JanProvaznik 6627e02
don't sign OpenTelemetry
JanProvaznik 86fcf77
sign correctly
JanProvaznik 039e929
thirdpartynotices new packages
JanProvaznik 83bdf4f
extensions primitives to swr
JanProvaznik 512ef64
more missing packages in swr
JanProvaznik da51175
remove otel assemblies from test project
JanProvaznik bfba539
gracefully handle load errors
JanProvaznik 3789e8b
reintroduce reliable shutdown in standalone, and unreliable longrunning
JanProvaznik c2d550b
fix tests
JanProvaznik a5c2ce6
avoid assembly loads when not sampled, add samplerate as a telemetry …
JanProvaznik b885721
refactor tests
JanProvaznik e4fe866
disable telemetry by default
JanProvaznik 26203bc
fix tests
JanProvaznik 81664cb
handle envvars correctly in test context
JanProvaznik 9e06c72
reformulate release checklist, needs hashing clarification, lock-free…
JanProvaznik 99f8d0a
Merge branch 'main' into otel-basic
JanProvaznik d4c024e
format thirdpartynotices
JanProvaznik 21c9d98
separate opt in and override sample rate concepts
JanProvaznik 034c1af
update telemetry version
JanProvaznik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
196 changes: 196 additions & 0 deletions
196
src/Build.UnitTests/BackEnd/OpenTelemetryActivities_Tests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,196 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Linq; | ||
using System.Text; | ||
using Xunit; | ||
using Shouldly; | ||
|
||
namespace Microsoft.Build.Framework.Telemetry.Tests | ||
{ | ||
public class ActivityExtensionsTests | ||
{ | ||
[Fact] | ||
public void WithTag_ShouldSetUnhashedValue() | ||
{ | ||
var activity = new Activity("TestActivity"); | ||
activity.Start(); | ||
|
||
var telemetryItem = new TelemetryItem( | ||
Name: "TestItem", | ||
Value: "TestValue", | ||
Hashed: false); | ||
|
||
activity.WithTag(telemetryItem); | ||
|
||
var tagValue = activity.GetTagItem("VS.MSBuild.TestItem"); | ||
tagValue.ShouldNotBeNull(); | ||
tagValue.ShouldBe("TestValue"); | ||
|
||
activity.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void WithTag_ShouldSetHashedValue() | ||
{ | ||
var activity = new Activity("TestActivity"); | ||
var telemetryItem = new TelemetryItem( | ||
Name: "TestItem", | ||
Value: "SensitiveValue", | ||
Hashed: true); | ||
|
||
activity.WithTag(telemetryItem); | ||
|
||
var tagValue = activity.GetTagItem("VS.MSBuild.TestItem"); | ||
tagValue.ShouldNotBeNull(); | ||
tagValue.ShouldNotBe("SensitiveValue"); // Ensure it’s not the plain text | ||
activity.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void WithTags_ShouldSetMultipleTags() | ||
{ | ||
var activity = new Activity("TestActivity"); | ||
var tags = new List<TelemetryItem> | ||
{ | ||
new("Item1", "Value1", false), | ||
new("Item2", "Value2", true) // hashed | ||
}; | ||
|
||
activity.WithTags(tags); | ||
|
||
var tagValue1 = activity.GetTagItem("VS.MSBuild.Item1"); | ||
var tagValue2 = activity.GetTagItem("VS.MSBuild.Item2"); | ||
|
||
tagValue1.ShouldNotBeNull(); | ||
tagValue1.ShouldBe("Value1"); | ||
|
||
tagValue2.ShouldNotBeNull(); | ||
tagValue2.ShouldNotBe("Value2"); // hashed | ||
|
||
activity.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void WithTags_DataHolderShouldSetMultipleTags() | ||
{ | ||
var activity = new Activity("TestActivity"); | ||
var dataHolder = new MockTelemetryDataHolder(); // see below | ||
|
||
activity.WithTags(dataHolder); | ||
|
||
var tagValueA = activity.GetTagItem("VS.MSBuild.TagA"); | ||
var tagValueB = activity.GetTagItem("VS.MSBuild.TagB"); | ||
|
||
tagValueA.ShouldNotBeNull(); | ||
tagValueA.ShouldBe("ValueA"); | ||
|
||
tagValueB.ShouldNotBeNull(); | ||
tagValueB.ShouldNotBe("ValueB"); // should be hashed | ||
activity.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void WithStartTime_ShouldSetActivityStartTime() | ||
{ | ||
var activity = new Activity("TestActivity"); | ||
var now = DateTime.UtcNow; | ||
|
||
activity.WithStartTime(now); | ||
|
||
activity.StartTimeUtc.ShouldBe(now); | ||
activity.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void WithStartTime_NullDateTime_ShouldNotSetStartTime() | ||
{ | ||
var activity = new Activity("TestActivity"); | ||
var originalStartTime = activity.StartTimeUtc; // should be default (min) if not started | ||
|
||
activity.WithStartTime(null); | ||
|
||
activity.StartTimeUtc.ShouldBe(originalStartTime); | ||
|
||
activity.Dispose(); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// A simple mock for testing IActivityTelemetryDataHolder. | ||
/// Returns two items: one hashed, one not hashed. | ||
/// </summary> | ||
internal sealed class MockTelemetryDataHolder : IActivityTelemetryDataHolder | ||
{ | ||
public IList<TelemetryItem> GetActivityProperties() | ||
{ | ||
return new List<TelemetryItem> | ||
{ | ||
new("TagA", "ValueA", false), | ||
new("TagB", "ValueB", true), | ||
}; | ||
} | ||
} | ||
|
||
|
||
public class MSBuildActivitySourceTests | ||
{ | ||
[Fact] | ||
public void StartActivity_ShouldPrefixNameCorrectly_WhenNoRemoteParent() | ||
{ | ||
var source = new MSBuildActivitySource(TelemetryConstants.DefaultActivitySourceNamespace); | ||
using var listener = new ActivityListener | ||
{ | ||
ShouldListenTo = activitySource => activitySource.Name == TelemetryConstants.DefaultActivitySourceNamespace, | ||
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData, | ||
}; | ||
ActivitySource.AddActivityListener(listener); | ||
|
||
|
||
var activity = source.StartActivity("Build"); | ||
|
||
activity.ShouldNotBeNull(); | ||
activity?.DisplayName.ShouldBe("VS/MSBuild/Build"); | ||
|
||
activity?.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void StartActivity_ShouldUseParentId_WhenRemoteParentExists() | ||
{ | ||
// Arrange | ||
var parentActivity = new Activity("ParentActivity"); | ||
parentActivity.SetParentId("|12345.abcde."); // Simulate some parent trace ID | ||
parentActivity.AddTag("sampleTag", "sampleVal"); | ||
parentActivity.Start(); | ||
|
||
var source = new MSBuildActivitySource(TelemetryConstants.DefaultActivitySourceNamespace); | ||
using var listener = new ActivityListener | ||
{ | ||
ShouldListenTo = activitySource => activitySource.Name == TelemetryConstants.DefaultActivitySourceNamespace, | ||
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData, | ||
}; | ||
ActivitySource.AddActivityListener(listener); | ||
|
||
// Act | ||
var childActivity = source.StartActivity("ChildBuild"); | ||
|
||
// Assert | ||
childActivity.ShouldNotBeNull(); | ||
// If HasRemoteParent is true, the code uses `parentId: Activity.Current.ParentId`. | ||
// However, by default .NET Activity doesn't automatically set HasRemoteParent = true | ||
// unless you explicitly set it. If you have logic that sets it, you can test it here. | ||
// For demonstration, we assume the ParentId is carried over if HasRemoteParent == true. | ||
if (Activity.Current?.HasRemoteParent == true) | ||
{ | ||
childActivity?.ParentId.ShouldBe("|12345.abcde."); | ||
} | ||
|
||
parentActivity.Dispose(); | ||
childActivity?.Dispose(); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it a temporary thing?
I recall we had some limitations for using this feed in msbuild repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's intended to be permanent. Under it, there is a restriction that this feed can pull only VS OpenTelemetry packages. (and it won't pull them outside Framework because they're conditionally included in Microsoft.Build.Framework.csproj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rainersigwald to draw your attention here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PSM configuration doesn't exactly say that: it says that
Microsoft.VisualStudio.OpenTelemetry*
must come fromvs-impl
, not thatvs-impl
can only be used for that package.We need to be careful here--have you talked to the VMR/sourcebuild folks about the dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I misunderstood that. Is it possible to do? From a quick look at docs I guess no☹️
We're in contact and it does not impact sourcebuild, because it's references are gated only for .NET Framework in Microsoft.Build.Framework.csproj, and .NET Framework dependencies are not a part of sourcebuild.
There is no way to condition on that in NuGet.config though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in dotnet-tools feed, removed vs-impl.cache trolled me