Skip to content

Commit 7350aac

Browse files
beeme1mrbeeme1mr
authored
feat: add all available attributes to a span (#167)
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** N/A: I can create one if necessary **Describe the solution you've provided** This PR adds the ability to capture all available attributes when in "span mode". It will only add the additional attributes when `IncludeAllAttributesInSpan` is `true`; otherwise, the existing behavior is retained. This change is useful because it's easier to work with spans than span events in some observability tools. With this change, spans can have the same attributes as span events. **Describe alternatives you've considered** We could leave it as is but it would make spans less useful to consumers. **Additional context** I talked to @kinyoklion about this on Slack about this change prior to opening the PR. Signed-off-by: Michael Beemer <[email protected]>
1 parent 902ac71 commit 7350aac

File tree

2 files changed

+116
-24
lines changed

2 files changed

+116
-24
lines changed

‎pkgs/telemetry/src/TracingHook.cs‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@ public class TracingHookBuilder
1616
{
1717
private bool _createActivities;
1818
private bool _includeValue;
19+
20+
private bool _includeAllAttributesInSpan;
1921
private string _environmentId;
2022

2123
internal TracingHookBuilder()
2224
{
2325
_createActivities = false;
2426
_includeValue = false;
27+
_includeAllAttributesInSpan = false;
2528
_environmentId = null;
2629
}
2730

@@ -66,6 +69,19 @@ public TracingHookBuilder IncludeVariant(bool includeVariant = true)
6669
return this;
6770
}
6871

72+
/// <summary>
73+
/// The TracingHook will include all attributes in the span if set to true.
74+
/// A span is only created if CreateActivities is set to true.
75+
/// This is disabled by default.
76+
/// </summary>
77+
/// <param name="includeAllAttributesInSpan">true to include all attributes, false otherwise</param>
78+
/// <returns>this builder</returns>
79+
public TracingHookBuilder IncludeAllAttributesInSpan(bool includeAllAttributesInSpan = true)
80+
{
81+
_includeAllAttributesInSpan = includeAllAttributesInSpan;
82+
return this;
83+
}
84+
6985
/// <summary>
7086
/// The environment ID associated with the SDK configuration. In typical usage the environment ID should not be
7187
/// specified. The environment ID only needs to be manually specified if it cannot be retrieved from the SDK.
@@ -90,7 +106,7 @@ public TracingHookBuilder EnvironmentId(string environmentId)
90106
/// <returns>the new hook</returns>
91107
public TracingHook Build()
92108
{
93-
return new TracingHook(new TracingHook.Options(_createActivities, _includeValue, _environmentId));
109+
return new TracingHook(new TracingHook.Options(_createActivities, _includeValue, _includeAllAttributesInSpan, _environmentId));
94110
}
95111
}
96112

@@ -133,12 +149,15 @@ internal struct Options
133149
public bool CreateActivities { get; }
134150
public bool IncludeValue { get; }
135151

152+
public bool IncludeAllAttributesInSpan { get; }
153+
136154
public string EnvironmentId { get; }
137155

138-
public Options(bool createActivities, bool includeValue, string environmentId = null)
156+
public Options(bool createActivities, bool includeValue, bool includeAllAttributesInSpan, string environmentId = null)
139157
{
140158
CreateActivities = createActivities;
141159
IncludeValue = includeValue;
160+
IncludeAllAttributesInSpan = includeAllAttributesInSpan;
142161
EnvironmentId = environmentId;
143162
}
144163
}
@@ -202,19 +221,6 @@ public override SeriesData BeforeEvaluation(EvaluationSeriesContext context, Ser
202221
/// <returns></returns>
203222
public override SeriesData AfterEvaluation(EvaluationSeriesContext context, SeriesData data, EvaluationDetail<LdValue> detail)
204223
{
205-
if (_options.CreateActivities && data.TryGetValue(ActivityFieldKey, out var value))
206-
{
207-
try
208-
{
209-
var activity = (Activity) value;
210-
activity?.Stop();
211-
}
212-
catch (System.InvalidCastException)
213-
{
214-
// This should never happen, but if it does, don't crash the application.
215-
}
216-
}
217-
218224
var attributes = new ActivityTagsCollection
219225
{
220226
{SemanticAttributes.FeatureFlagKey, context.FlagKey},
@@ -246,6 +252,29 @@ public override SeriesData AfterEvaluation(EvaluationSeriesContext context, Seri
246252
attributes.Add(SemanticAttributes.FeatureFlagVariationIndex, detail.VariationIndex.Value);
247253
}
248254

255+
if (_options.CreateActivities && data.TryGetValue(ActivityFieldKey, out var value))
256+
{
257+
try
258+
{
259+
var activity = (Activity)value;
260+
if (activity != null)
261+
{
262+
if (activity.IsAllDataRequested == true && _options.IncludeAllAttributesInSpan)
263+
{
264+
foreach (var kvp in attributes)
265+
{
266+
activity.SetTag(kvp.Key, kvp.Value);
267+
}
268+
}
269+
activity.Stop();
270+
}
271+
}
272+
catch (System.InvalidCastException)
273+
{
274+
// This should never happen, but if it does, don't crash the application.
275+
}
276+
}
277+
249278
Activity.Current?.AddEvent(new ActivityEvent(name: SemanticAttributes.EventName, tags: attributes));
250279
return data;
251280
}

‎pkgs/telemetry/test/TracingHookTests.cs‎

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ public void CallingDisposeDoesNotThrowException()
5656
}
5757

5858
[Theory]
59-
[InlineData(true)]
60-
[InlineData(false)]
61-
public void TracingHookCreatesRootSpans(bool createSpans)
59+
[InlineData(true, false)]
60+
[InlineData(true, true)]
61+
[InlineData(false, false)]
62+
[InlineData(false, true)]
63+
public void TracingHookCreatesRootSpans(bool createSpans, bool includeAllAttributesInSpan)
6264
{
6365
ICollection<Activity> exportedItems = new Collection<Activity>();
6466

@@ -67,7 +69,11 @@ public void TracingHookCreatesRootSpans(bool createSpans)
6769
.AddInMemoryExporter(exportedItems)
6870
.Build();
6971

70-
var hookUnderTest = TracingHook.Builder().CreateActivities(createSpans).Build();
72+
var hookUnderTest = TracingHook.Builder()
73+
.CreateActivities(createSpans)
74+
.IncludeValue(true)
75+
.IncludeAllAttributesInSpan(includeAllAttributesInSpan)
76+
.Build();
7177
var featureKey = "feature-key";
7278
var context = Context.New("foo");
7379

@@ -93,6 +99,30 @@ public void TracingHookCreatesRootSpans(bool createSpans)
9399
Assert.Equal("LdClient.BoolVariation", items[0].OperationName);
94100
Assert.Equal("LdClient.StringVariation", items[1].OperationName);
95101
Assert.True(items.All(i => i.Parent == null));
102+
103+
Assert.All(items, activity =>
104+
{
105+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.key" && tag.Value.ToString() == featureKey);
106+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.context.id" && tag.Value.ToString() == context.FullyQualifiedKey);
107+
});
108+
109+
// Verify span attributes based on includeAllAttributesInSpan setting
110+
if (includeAllAttributesInSpan)
111+
{
112+
Assert.All(items, activity =>
113+
{
114+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.provider.name" && tag.Value.ToString() == "LaunchDarkly");
115+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.result.value");
116+
});
117+
}
118+
else
119+
{
120+
// When includeAllAttributesInSpan is false, only the initial attributes from BeforeEvaluation should be present
121+
Assert.All(items, activity =>
122+
{
123+
Assert.DoesNotContain(activity.Tags, tag => tag.Key == "feature_flag.result.value");
124+
});
125+
}
96126
}
97127
else
98128
{
@@ -103,9 +133,11 @@ public void TracingHookCreatesRootSpans(bool createSpans)
103133

104134

105135
[Theory]
106-
[InlineData(true)]
107-
[InlineData(false)]
108-
public void TracingHookCreatesChildSpans(bool createSpans)
136+
[InlineData(true, true)]
137+
[InlineData(true, false)]
138+
[InlineData(false, true)]
139+
[InlineData(false, false)]
140+
public void TracingHookCreatesChildSpans(bool createSpans, bool includeAllAttributesInSpan)
109141
{
110142
ICollection<Activity> exportedItems = new Collection<Activity>();
111143

@@ -120,7 +152,11 @@ public void TracingHookCreatesChildSpans(bool createSpans)
120152
.AddInMemoryExporter(exportedItems)
121153
.Build();
122154

123-
var hookUnderTest = TracingHook.Builder().CreateActivities(createSpans).Build();
155+
var hookUnderTest = TracingHook.Builder()
156+
.CreateActivities(createSpans)
157+
.IncludeValue(true)
158+
.IncludeAllAttributesInSpan(includeAllAttributesInSpan)
159+
.Build();
124160
var featureKey = "feature-key";
125161
var context = Context.New("foo");
126162

@@ -151,6 +187,34 @@ public void TracingHookCreatesChildSpans(bool createSpans)
151187
Assert.Equal("LdClient.StringVariation", items[1].OperationName);
152188
Assert.Equal("root-activity", items[2].OperationName);
153189
Assert.Equal(items[2].SpanId, items[0].ParentSpanId);
190+
191+
// Get only the LaunchDarkly child spans (not the root activity)
192+
var childSpans = items.Where(i => i.OperationName != "root-activity").ToList();
193+
194+
// Verify initial attributes are always present on child spans
195+
Assert.All(childSpans, activity =>
196+
{
197+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.key" && tag.Value.ToString() == featureKey);
198+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.context.id" && tag.Value.ToString() == context.FullyQualifiedKey);
199+
});
200+
201+
// Verify span attributes based on includeAllAttributesInSpan setting
202+
if (includeAllAttributesInSpan)
203+
{
204+
Assert.All(childSpans, activity =>
205+
{
206+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.provider.name" && tag.Value.ToString() == "LaunchDarkly");
207+
Assert.Contains(activity.Tags, tag => tag.Key == "feature_flag.result.value");
208+
});
209+
}
210+
else
211+
{
212+
// When includeAllAttributesInSpan is false, only the initial attributes from BeforeEvaluation should be present
213+
Assert.All(childSpans, activity =>
214+
{
215+
Assert.DoesNotContain(activity.Tags, tag => tag.Key == "feature_flag.result.value");
216+
});
217+
}
154218
}
155219
else
156220
{
@@ -221,7 +285,6 @@ public void TracingHookIncludesVariant(bool includeValue)
221285
}
222286
}
223287

224-
225288
[Fact]
226289
public void TracingHookIncludesEnvironmentIdWhenSpecified()
227290
{

0 commit comments

Comments
 (0)