Skip to content

Commit c493a0d

Browse files
author
Liudmila Molkova
committed
address comments, add tests
1 parent 2b8d9ba commit c493a0d

File tree

5 files changed

+125
-65
lines changed

5 files changed

+125
-65
lines changed

‎src/OpenTelemetry.Api/Trace/TracerProviderBuilder.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,6 @@ public abstract TracerProviderBuilder AddInstrumentation<TInstrumentation>(
3636
/// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
3737
public abstract TracerProviderBuilder AddSource(params string[] names);
3838

39-
/// <summary>
40-
/// Adds filter for <see cref="ActivitySource"/> to subscribe to.
41-
/// </summary>
42-
/// <param name="sourceFilter">Activity Source filter - if it returns true, OpenTelemetry subscribes to the corresponding source.</param>
43-
/// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
44-
public abstract TracerProviderBuilder AddSource(Func<ActivitySource, bool> sourceFilter);
45-
4639
/// <summary>
4740
/// Adds a listener for <see cref="Activity"/> objects created with the given operation name to the <see cref="TracerProviderBuilder"/>.
4841
/// </summary>

‎src/OpenTelemetry/Trace/Builder/TracerProviderBuilderExtensions.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,27 @@ public static TracerProviderBuilder AddProcessor(
231231
return tracerProviderBuilder;
232232
}
233233

234+
/// <summary>
235+
/// Adds predicate for <see cref="ActivitySource"/> that will be used to determine
236+
/// if <see cref="TracerProvider"/> should subscribe to the source.
237+
/// </summary>
238+
/// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param>
239+
/// <param name="sourcePredicate">Activity Source predicate - if it returns true, OpenTelemetry subscribes to the corresponding source.</param>
240+
/// <returns>Returns <see cref="TracerProviderBuilder"/> for chaining.</returns>
241+
public static TracerProviderBuilder AddSource(this TracerProviderBuilder tracerProviderBuilder, Predicate<ActivitySource> sourcePredicate)
242+
{
243+
Guard.ThrowIfNull(sourcePredicate);
244+
tracerProviderBuilder.ConfigureBuilder((_, builder) =>
245+
{
246+
if (builder is TracerProviderBuilderSdk tracerProviderBuilderSdk)
247+
{
248+
tracerProviderBuilderSdk.AddSource(sourcePredicate);
249+
}
250+
});
251+
252+
return tracerProviderBuilder;
253+
}
254+
234255
/// <summary>
235256
/// Run the given actions to initialize the <see cref="TracerProvider"/>.
236257
/// </summary>

‎src/OpenTelemetry/Trace/Builder/TracerProviderBuilderSdk.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public TracerProviderBuilderSdk(IServiceProvider serviceProvider)
3333

3434
public List<string> Sources { get; } = new();
3535

36-
public Func<ActivitySource, bool>? SourceFilter { get; private set; } = null;
36+
public List<Predicate<ActivitySource>> SourceSelectionPredicates { get; } = new();
3737

3838
public HashSet<string> LegacyActivityOperationNames { get; } = new(StringComparer.OrdinalIgnoreCase);
3939

@@ -125,10 +125,10 @@ public override TracerProviderBuilder AddSource(params string[] names)
125125
return this;
126126
}
127127

128-
public override TracerProviderBuilder AddSource(Func<ActivitySource, bool> sourceFilter)
128+
public TracerProviderBuilder AddSource(Predicate<ActivitySource> sourcePredicate)
129129
{
130-
Debug.Assert(sourceFilter != null, "sourceFilter was null");
131-
this.SourceFilter = sourceFilter;
130+
Debug.Assert(sourcePredicate != null, "sourcePredicate was null");
131+
this.SourceSelectionPredicates.Add(sourcePredicate);
132132
return this;
133133
}
134134

‎src/OpenTelemetry/Trace/TracerProviderSdk.cs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -242,46 +242,44 @@ internal TracerProviderSdk(
242242
this.getRequestedDataAction = this.RunGetRequestedDataOtherSampler;
243243
}
244244

245-
listener.ShouldListenTo = this.GetFilter(state);
245+
listener.ShouldListenTo = this.GetPredicate(state);
246246
ActivitySource.AddActivityListener(listener);
247247
this.listener = listener;
248248
OpenTelemetrySdkEventSource.Log.TracerProviderSdkEvent("TracerProvider built successfully.");
249249
}
250250

251-
private Func<ActivitySource, bool>? GetFilter(TracerProviderBuilderSdk state)
251+
private Func<ActivitySource, bool> GetPredicate(TracerProviderBuilderSdk state)
252252
{
253+
List<Predicate<ActivitySource>> predicates = new List<Predicate<ActivitySource>>();
254+
253255
// Sources can be empty. This happens when user
254256
// is only interested in InstrumentationLibraries
255257
// which do not depend on ActivitySources.
256-
List<Func<ActivitySource, bool>> filters = new List<Func<ActivitySource, bool>>();
257258
if (state.Sources.Any())
258259
{
259-
filters.Add(this.GetNameFilter(state));
260+
predicates.Add(this.GetNameFilter(state));
260261
}
261262

262-
if (state.SourceFilter != null)
263-
{
264-
filters.Add(state.SourceFilter);
265-
}
263+
predicates.AddRange(state.SourceSelectionPredicates);
266264

267265
if (this.supportLegacyActivity)
268266
{
269-
filters.Add((activitySource) => string.IsNullOrEmpty(activitySource.Name));
267+
predicates.Add((activitySource) => string.IsNullOrEmpty(activitySource.Name));
270268
}
271269

272270
return (activitySource) =>
273271
{
274272
bool shouldListen = false;
275-
for (int i = 0; i < filters.Count && !shouldListen; i++)
273+
for (int i = 0; i < predicates.Count && !shouldListen; i++)
276274
{
277-
shouldListen |= filters[i](activitySource);
275+
shouldListen |= predicates[i](activitySource);
278276
}
279277

280278
return shouldListen;
281279
};
282280
}
283281

284-
private Func<ActivitySource, bool> GetNameFilter(TracerProviderBuilderSdk state)
282+
private Predicate<ActivitySource> GetNameFilter(TracerProviderBuilderSdk state)
285283
{
286284
Debug.Assert(state.Sources.Any(), "Should only be called when there are name-based source filters.");
287285

‎test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs

Lines changed: 90 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,8 @@ public void TracerProviderSdkAddSource()
2727
.AddSource(source1.Name)
2828
.Build();
2929

30-
using (var activity = source1.StartActivity("test"))
31-
{
32-
Assert.NotNull(activity);
33-
}
34-
35-
using (var activity = source2.StartActivity("test"))
36-
{
37-
Assert.Null(activity);
38-
}
30+
Assert.True(IsSourceEnabled(source1));
31+
Assert.False(IsSourceEnabled(source2));
3932
}
4033

4134
[Fact]
@@ -50,50 +43,97 @@ public void TracerProviderSdkAddSourceWithWildcards()
5043
.AddSource($"{Utils.GetCurrentMethodName()}.*")
5144
.Build())
5245
{
53-
using (var activity = source1.StartActivity("test"))
54-
{
55-
Assert.NotNull(activity);
56-
}
46+
Assert.True(IsSourceEnabled(source1));
47+
Assert.True(IsSourceEnabled(source2));
48+
Assert.True(IsSourceEnabled(source3));
49+
Assert.True(IsSourceEnabled(source4));
50+
}
5751

58-
using (var activity = source2.StartActivity("test"))
59-
{
60-
Assert.NotNull(activity);
61-
}
52+
using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
53+
.AddSource($"{Utils.GetCurrentMethodName()}.?")
54+
.Build())
55+
{
56+
Assert.True(IsSourceEnabled(source1));
57+
Assert.False(IsSourceEnabled(source2));
58+
Assert.False(IsSourceEnabled(source3));
59+
Assert.True(IsSourceEnabled(source4));
60+
}
61+
}
6262

63-
using (var activity = source3.StartActivity("test"))
64-
{
65-
Assert.NotNull(activity);
66-
}
63+
[Fact]
64+
public void TracerProviderSdkAddSourceWithPredicate()
65+
{
66+
using var source1 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "1.0.0");
67+
using var source2 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "2.0.0");
68+
using var source3 = new ActivitySource($"{Utils.GetCurrentMethodName()}.B");
69+
using var source4 = new ActivitySource($"B.{Utils.GetCurrentMethodName()}");
6770

68-
using (var activity = source4.StartActivity("test"))
69-
{
70-
Assert.NotNull(activity);
71-
}
71+
using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
72+
.AddSource(source => source.Version == "2.0.0" || source.Name.EndsWith(".B"))
73+
.Build())
74+
{
75+
Assert.False(IsSourceEnabled(source1));
76+
Assert.True(IsSourceEnabled(source2));
77+
Assert.True(IsSourceEnabled(source3));
78+
Assert.False(IsSourceEnabled(source4));
7279
}
80+
}
81+
82+
[Fact]
83+
public void TracerProviderSdkAddSourceWithMultiplePredicates()
84+
{
85+
using var source1 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "1.0.0");
86+
using var source2 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "2.0.0");
87+
using var source3 = new ActivitySource($"{Utils.GetCurrentMethodName()}.B");
88+
using var source4 = new ActivitySource($"B.{Utils.GetCurrentMethodName()}");
7389

7490
using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
75-
.AddSource($"{Utils.GetCurrentMethodName()}.?")
91+
.AddSource(source => source.Version == "2.0.0")
92+
.AddSource(source => source.Name.StartsWith("B."))
7693
.Build())
7794
{
78-
using (var activity = source1.StartActivity("test"))
79-
{
80-
Assert.NotNull(activity);
81-
}
95+
Assert.False(IsSourceEnabled(source1));
96+
Assert.True(IsSourceEnabled(source2));
97+
Assert.False(IsSourceEnabled(source3));
98+
Assert.True(IsSourceEnabled(source4));
99+
}
100+
}
82101

83-
using (var activity = source2.StartActivity("test"))
84-
{
85-
Assert.Null(activity);
86-
}
102+
[Fact]
103+
public void TracerProviderSdkAddSourceWithWildCardAndPredicate()
104+
{
105+
using var source1 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "1.0.0");
106+
using var source2 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "2.0.0");
107+
using var source3 = new ActivitySource($"{Utils.GetCurrentMethodName()}.B");
108+
using var source4 = new ActivitySource($"B.{Utils.GetCurrentMethodName()}");
87109

88-
using (var activity = source3.StartActivity("test"))
89-
{
90-
Assert.Null(activity);
91-
}
110+
using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
111+
.AddSource("B.*")
112+
.AddSource(source => source.Version == "2.0.0")
113+
.Build())
114+
{
115+
Assert.False(IsSourceEnabled(source1));
116+
Assert.True(IsSourceEnabled(source2));
117+
Assert.False(IsSourceEnabled(source3));
118+
Assert.True(IsSourceEnabled(source4));
119+
}
120+
}
92121

93-
using (var activity = source4.StartActivity("test"))
94-
{
95-
Assert.NotNull(activity);
96-
}
122+
[Fact]
123+
public void TracerProviderSdkAddSourceWithConflictingWildCardAndPredicate()
124+
{
125+
using var source1 = new ActivitySource($"{Utils.GetCurrentMethodName()}.A", "1.0.0");
126+
using var source2 = new ActivitySource($"{Utils.GetCurrentMethodName()}.B");
127+
using var source3 = new ActivitySource($"B.{Utils.GetCurrentMethodName()}");
128+
129+
using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
130+
.AddSource("B.*")
131+
.AddSource(source => !source.Name.StartsWith("B."))
132+
.Build())
133+
{
134+
Assert.True(IsSourceEnabled(source1));
135+
Assert.True(IsSourceEnabled(source2));
136+
Assert.True(IsSourceEnabled(source3));
97137
}
98138
}
99139

@@ -1242,6 +1282,14 @@ public void Dispose()
12421282
GC.SuppressFinalize(this);
12431283
}
12441284

1285+
private static bool IsSourceEnabled(ActivitySource source)
1286+
{
1287+
using (var activity = source.StartActivity("test"))
1288+
{
1289+
return activity != null;
1290+
}
1291+
}
1292+
12451293
private sealed class TestTracerProviderBuilder : TracerProviderBuilderBase
12461294
{
12471295
public TracerProviderBuilder AddInstrumentation()

0 commit comments

Comments
 (0)