Skip to content

Conversation

@TimothyMothra
Copy link

@TimothyMothra TimothyMothra commented Feb 10, 2023

I found a bug in the way we serialize some arrays stored Activity Tags.

For example:

using (var activity = activitySource.StartActivity(name: "SayHello", kind: activityKind))
{
    activity?.SetTag("intArray", new int[] { 1, 2, 3 });
}

this would be serialized as key: "intArray" value: "1,2,3".

However,

using (var activity = activitySource.StartActivity(name: "ActivityWithException"))
{
	var eventTags = new Dictionary<string, object>
	{
		{ "intArray", new int[] { 1, 2, 3 } }
	};
	activity?.AddEvent(new("Gonna try it!", DateTimeOffset.Now, new(eventTags)));
}

this is being serialized as key: "intArray" value: "System.Int32[]"

The fix here is to consistently check for Array type and serialize the component items.

Changes

  • Add extension method Array.ToCommaDelimitedString() to serialize an Array as a comma delimited string.
    This copies the implementation from AzMonList's TagEnumerationState to use in the TraceHelper.
  • Refactor unit tests
  • Added a unit test for ActivityEvent.

Screenshot

image

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@TimothyMothra TimothyMothra merged commit 5db3f5b into main Feb 11, 2023
@TimothyMothra TimothyMothra deleted the tilee/arrayToString branch February 11, 2023 00:31
glharper added a commit that referenced this pull request Feb 15, 2023
Co-authored-by: Jesse Squire <[email protected]>
Co-authored-by: archerzz <[email protected]>
Co-authored-by: Reiley Yang <[email protected]>
Co-authored-by: Jesse Squire <[email protected]>
Co-authored-by: HarmanDhunna <[email protected]>
Co-authored-by: HarmanDhunna <[email protected]>
Co-authored-by: Richard Park <[email protected]>
Co-authored-by: Mike Harder <[email protected]>
Co-authored-by: Theodore Chang <[email protected]>
Co-authored-by: Arcturus Zhang <[email protected]>
Co-authored-by: Wes Haggard <[email protected]>
Co-authored-by: Yao Kou <[email protected]>
Co-authored-by: ArcturusZhang <[email protected]>
Co-authored-by: Vishwesh Bankwar <[email protected]>
Co-authored-by: JoshLove-msft <[email protected]>
Co-authored-by: Scott Addie <[email protected]>
Co-authored-by: Madalyn Redding <[email protected]>
Co-authored-by: Anne Thompson <[email protected]>
Co-authored-by: Jose Arriaga Maldonado <[email protected]>
Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: Scott Schaab <[email protected]>
Co-authored-by: Heath Stewart <[email protected]>
Co-authored-by: ShivangiReja <[email protected]>
Co-authored-by: Wes Haggard <[email protected]>
Co-authored-by: Timothy Mothra <[email protected]>
Co-authored-by: ShaneMicro <[email protected]>
Co-authored-by: Feng Zhou <[email protected]>
Co-authored-by: nisha-bhatia <[email protected]>
Co-authored-by: Nathan Carlson <[email protected]>
Co-authored-by: wangrui-msft <[email protected]>
Co-authored-by: Theodore Chang <[email protected]>
Co-authored-by: Arthur Ma <[email protected]>
Co-authored-by: isolenov <[email protected]>
Co-authored-by: m-nash <[email protected]>
Co-authored-by: Konrad Jamrozik <[email protected]>
Co-authored-by: Caio Saldanha <[email protected]>
Co-authored-by: Chengming <[email protected]>
Co-authored-by: Minghao Chen <[email protected]>
Co-authored-by: Alexey Rodionov <[email protected]>
Co-authored-by: Travis Wilson <[email protected]>
Fix typo in Identity changelog (#34022)
fix logic workflow update method (#33944)
fixes #33975 by reformatting snippets in sdk/core.
fix datetimeoffset format string (#34031)
Fix creat recognize request of call automation media (#34036)
Fix how arrays stored in Activity Tags are serialized. (#34086)
fix more nullables (#34092)
resolver, logic, storage (#34140)
fix MSI deserialization (#34136)
resolver releases (#34144)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Monitor - Exporter Monitor OpenTelemetry Exporter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants