Skip to content

Conversation

@ablyler
Copy link

@ablyler ablyler commented Nov 21, 2025

  • Introduced propertiesAsStructuredMetadata to extract high-cardinality data as structured metadata.
  • Enhanced LokiBatchFormatter and Loki serialization model to handle structured metadata.
  • Updated documentation and examples for structured metadata integration.
  • Adjusted tests to verify proper metadata serialization behavior.

Refs #255

Copilot AI review requested due to automatic review settings November 21, 2025 20:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds support for Loki structured metadata, enabling high-cardinality data (like trace IDs, user IDs, request IDs) to be attached to log entries without being indexed as labels. The implementation introduces new configuration parameters propertiesAsStructuredMetadata and leaveStructuredMetadataPropertiesIntact to control which properties are extracted as structured metadata and whether they should remain in the log entry JSON.

Key Changes:

  • Enhanced the Loki serialization model to support structured metadata as a third element in log entry arrays
  • Added extraction logic in LokiBatchFormatter to process properties designated as structured metadata
  • Extended API with new configuration parameters for structured metadata handling

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Serilog.Sinks.Grafana.Loki/Models/LokiStream.cs Changed Entries from IList<IList<string>> to IList<IList<object>> and added overload to accept structured metadata dictionary as third element
src/Serilog.Sinks.Grafana.Loki/Models/LokiSerializationContext.cs Added JSON serialization support for Dictionary<string, string>, object[], and string types
src/Serilog.Sinks.Grafana.Loki/LokiBatchFormatter.cs Implemented structured metadata extraction logic with property removal support and new constructor parameters
src/Serilog.Sinks.Grafana.Loki/LoggerConfigurationLokiExtensions.cs Extended API with propertiesAsStructuredMetadata and leaveStructuredMetadataPropertiesIntact parameters
test/Serilog.Sinks.Grafana.Loki.Tests/IntegrationTests/LokiJsonTextFormatterRequestPayloadTests.cs Added integration tests for structured metadata serialization with and without property retention
sample/Serilog.Sinks.Grafana.Loki.SampleWebApp/appsettings.json Added example configuration showing RequestId and RequestPath as structured metadata
README.md Comprehensive documentation for structured metadata feature including configuration examples, requirements, and usage patterns
src/Serilog.Sinks.Grafana.Loki/Serilog.Sinks.Grafana.Loki.csproj Modified package ID and repository URLs (appears unrelated to the feature)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +194 to +215
// Extract structured metadata
Dictionary<string, string>? structuredMetadata = null;
if (_propertiesAsStructuredMetadata.Any())
{
structuredMetadata = new Dictionary<string, string>();
var propertiesToExtract = logEvent.Properties
.Where(kvp => _propertiesAsStructuredMetadata.Contains(kvp.Key))
.ToList();

foreach (var property in propertiesToExtract)
{
// Remove quotes from the value string representation
var value = property.Value.ToString().Replace("\"", string.Empty);
structuredMetadata[property.Key] = value;

// Remove the property from the log event if configured to do so
if (!_leaveStructuredMetadataPropertiesIntact)
{
logEvent.RemovePropertyIfPresent(property.Key);
}
}
}
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

Unnecessary dictionary allocation when no properties need to be extracted as structured metadata. Consider initializing the dictionary only when properties are actually found:

Dictionary<string, string>? structuredMetadata = null;
var propertiesToExtract = logEvent.Properties
    .Where(kvp => _propertiesAsStructuredMetadata.Contains(kvp.Key))
    .ToList();

if (propertiesToExtract.Any())
{
    structuredMetadata = new Dictionary<string, string>();
    foreach (var property in propertiesToExtract)
    {
        var value = property.Value.ToString().Replace("\"", string.Empty);
        structuredMetadata[property.Key] = value;

        if (!_leaveStructuredMetadataPropertiesIntact)
        {
            logEvent.RemovePropertyIfPresent(property.Key);
        }
    }
}

This avoids creating an empty dictionary on every log entry when none of the structured metadata properties are present.

Copilot uses AI. Check for mistakes.
Comment on lines 15 to 20
<RepositoryUrl>https://github.com/ablyler/serilog-sinks-grafana-loki.git</RepositoryUrl>
<!--NuGet package-->
<PackageIcon>serilog-sink-nuget.png</PackageIcon>
<PackageId>Serilog.Sinks.Grafana.Loki</PackageId>
<PackageId>AndyBlyler.Sinks.Grafana.Loki</PackageId>
<PackageLicenseFile>LICENSE</PackageLicenseFile>
<PackageProjectUrl>https://github.com/serilog-contrib/serilog-sinks-grafana-loki</PackageProjectUrl>
<PackageProjectUrl>https://github.com/ablyler/serilog-sinks-grafana-loki</PackageProjectUrl>
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

Package configuration changes appear to be unrelated to the structured metadata feature. The changes to PackageId (from Serilog.Sinks.Grafana.Loki to AndyBlyler.Sinks.Grafana.Loki) and repository URLs (from serilog-contrib to ablyler) suggest this might be a fork or personal build configuration that should not be part of this pull request.

Copilot uses AI. Check for mistakes.
Comment on lines 60 to +83
public LokiBatchFormatter(
IReservedPropertyRenamingStrategy renamingStrategy,
IEnumerable<LokiLabel>? globalLabels = null,
IEnumerable<string>? propertiesAsLabels = null,
IEnumerable<string>? propertiesAsStructuredMetadata = null,
bool useInternalTimestamp = false,
bool leavePropertiesIntact = false)
bool leavePropertiesIntact = false,
bool leaveStructuredMetadataPropertiesIntact = false)
{
_renamingStrategy = renamingStrategy;
_globalLabels = globalLabels ?? Enumerable.Empty<LokiLabel>();
_propertiesAsLabels = propertiesAsLabels ?? Enumerable.Empty<string>();
_propertiesAsStructuredMetadata = propertiesAsStructuredMetadata ?? Enumerable.Empty<string>();
_useInternalTimestamp = useInternalTimestamp;
_leavePropertiesIntact = leavePropertiesIntact;
_leaveStructuredMetadataPropertiesIntact = leaveStructuredMetadataPropertiesIntact;
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

[nitpick] There's no validation to prevent a property from being specified in both propertiesAsLabels and propertiesAsStructuredMetadata. If a property appears in both lists, it will be:

  1. First extracted as a label in GenerateLabels (line 131) and removed from properties if leavePropertiesIntact is false
  2. Then attempted to be extracted as structured metadata in GenerateEntry (line 149), but the property may have already been removed

Consider adding validation in the constructor or documentation to clarify the expected behavior when properties overlap between these two lists, or handle this case explicitly by checking if a property has already been removed.

Copilot uses AI. Check for mistakes.
Loki supports [structured metadata](https://grafana.com/docs/loki/latest/reference/loki-http-api/#ingest-logs) - additional key-value pairs that can be attached to each log line without being indexed as labels. This is useful for high-cardinality data like trace IDs, user IDs, or request IDs that you want to query but don't want to use as labels.

> **Requirements:**
> - Structured metadata requires **Loki 1.2.0 or newer**
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

The version requirement "Loki 1.2.0 or newer" appears to be incorrect. Structured metadata support was introduced in Loki 2.4.0 (released in 2022). Please verify and update the documentation to reflect the correct minimum version requirement.

Reference: Grafana Loki's structured metadata feature was introduced in version 2.4.0.

Suggested change
> - Structured metadata requires **Loki 1.2.0 or newer**
> - Structured metadata requires **Loki 2.4.0 or newer**

Copilot uses AI. Check for mistakes.
- Introduced `propertiesAsStructuredMetadata` to extract high-cardinality data as structured metadata.
- Enhanced `LokiBatchFormatter` and Loki serialization model to handle structured metadata.
- Updated documentation and examples for structured metadata integration.
- Adjusted tests to verify proper metadata serialization behavior.

Signed-off-by: Andy Blyler <[email protected]>
@ablyler ablyler force-pushed the feature/255-structured-metadata-support branch from fc42951 to 7478275 Compare November 22, 2025 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant