Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 17, 2025

Plan: Support Complex Properties as Custom Dimensions ✅

The issue is that complex properties (objects, nested structures) are not being serialized properly in Application Insights custom dimensions. Currently, StringDictionaryConverter.SafeValueConverter() only calls Convert.ToString() which results in type names instead of actual property values.

Tasks:

  • Review existing code and understand the issue with SafeValueConverter method
  • Add System.Text.Json dependency to the project
  • Update SafeValueConverter to serialize complex objects to JSON
  • Create tests to validate complex object serialization
  • Run tests to ensure all existing tests pass
  • Run code review and security checks
  • Verify the fix works correctly with nested objects, arrays, etc.
  • Upgrade System.Text.Json to version 10.0.0 (per review feedback)
  • Fix merge conflict - ensure TryAddPropertyToPropertyBag uses JSON serialization
  • Merge dev branch and update lockfile for .NET 10 support

Changes Made:

  1. Added System.Text.Json dependency (version 10.0.0) to enable JSON serialization
    • System.Text.Json 10.0.0 supports .NET Standard 2.0, which includes .NET 6.0+
  2. Updated StringDictionaryConverter.SafeValueConverter() to intelligently handle different value types:
    • Null values: Returns empty string
    • Strings: Returns directly without conversion
    • Primitive types (int, bool, char, etc.), enums, decimal, DateTime, DateTimeOffset, Guid, TimeSpan: Uses Convert.ToString() with InvariantCulture
    • Complex objects (objects with properties, arrays, dictionaries, etc.): Serializes to JSON using System.Text.Json.JsonSerializer
  3. Fixed merge conflict: Updated TryAddPropertyToPropertyBag method to use StringDictionaryConverter instead of Convert.ToString() to ensure complex properties are serialized as JSON
  4. Merged latest dev changes: Integrated .NET 10 support and updated lockfile
  5. Added comprehensive tests:
    • TraceHasComplexPropertiesSerializedAsJson: Validates complex objects with nested properties and arrays are properly serialized to JSON
    • TraceHandlesVariousPropertyTypes: Tests edge cases including strings, primitives, arrays, dictionaries, and null values
  6. All tests pass (50 passed, 1 skipped) on net8.0, net9.0, and net10.0, ensuring backward compatibility

Security:

  • ✅ CodeQL analysis found 0 security vulnerabilities
  • Safe JSON serialization with error handling (returns empty string on failure)

Impact:

  • Complex properties now properly appear in Application Insights custom dimensions as readable JSON instead of type names
  • Backward compatible - simple types continue to work as before
  • Works correctly with the new property collision handling from the merge
  • Now supports .NET 10 test target
Original prompt

This section details on the original issue you should resolve

<issue_title>Complex properties as customDimensions</issue_title>
<issue_description>Currently, all properties are only stored in one level, ie. complex properties do not seem to get serialized.

private static string SafeValueConverter(object value)
{
try
{
return Convert.ToString(value, System.Globalization.CultureInfo.InvariantCulture);
}
catch
{
return string.Empty;
}
}
</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Fix serialization for complex properties in customDimensions Serialize complex properties as JSON in Application Insights custom dimensions Nov 17, 2025
Copilot AI requested a review from hangy November 17, 2025 15:45
@hangy hangy changed the title Serialize complex properties as JSON in Application Insights custom dimensions feat: Serialize complex properties as JSON in Application Insights custom dimensions Nov 17, 2025
@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.05%. Comparing base (4533191) to head (ff040f5).
⚠️ Report is 6 commits behind head on dev.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev      #76      +/-   ##
==========================================
+ Coverage   60.80%   65.05%   +4.24%     
==========================================
  Files           3        3              
  Lines         273      289      +16     
  Branches       49       52       +3     
==========================================
+ Hits          166      188      +22     
+ Misses         86       80       -6     
  Partials       21       21              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hangy hangy marked this pull request as ready for review November 17, 2025 18:36
Copilot AI requested a review from hangy November 17, 2025 18:36
Copy link
Owner

@hangy hangy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Copilot AI commented Nov 17, 2025

@copilot Address the build error: https://github.com/hangy/NLog.Targets.ApplicationInsights/actions/runs/19440496628/job/55622368888?pr=76

Fixed in commit d5eb2ba. The build error was caused by a merge conflict where the new TryAddPropertyToPropertyBag method was using Convert.ToString() instead of StringDictionaryConverter, bypassing the JSON serialization logic. System.Text.Json 10.0.0 is correct as it supports .NET Standard 2.0 (which includes .NET 6.0+). All tests now pass.

Copilot AI requested a review from hangy November 17, 2025 18:53
@hangy
Copy link
Owner

hangy commented Nov 17, 2025

@copilot There's now a merge conflict. Fix it by updating the lockfile

Copy link
Contributor Author

Copilot AI commented Nov 17, 2025

@copilot There's now a merge conflict. Fix it by updating the lockfile

Merge conflict resolved in commit ff040f5. Updated the lockfile by accepting dev branch changes and regenerating with dotnet restore. The branch now includes .NET 10 support (dropping .NET 6.0). All tests pass on net8.0, net9.0, and net10.0.

@hangy hangy merged commit 4894621 into dev Nov 17, 2025
9 checks passed
@hangy hangy deleted the copilot/fix-complex-properties-serialization branch November 17, 2025 19:24
github-actions bot pushed a commit that referenced this pull request Jan 1, 2026
# [1.2.0-beta.7](v1.2.0-beta.6...v1.2.0-beta.7) (2026-01-01)

### Bug Fixes

* Property collision handling: ensure event properties take precedence over context properties ([#75](#75)) ([211f262](211f262))

### Features

* Serialize complex properties as JSON in Application Insights custom dimensions ([#76](#76)) ([4894621](4894621))
@github-actions
Copy link

github-actions bot commented Jan 1, 2026

🎉 This PR is included in version 1.2.0-beta.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Complex properties as customDimensions

2 participants