Skip to content

Support custom LogEventPropertyValue values in direct log submission #8034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrewlock merged 2 commits into from
Jan 8, 2026
Merged

Support custom LogEventPropertyValue values in direct log submission #8034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab67c8b
Support custom LogEventPropertyValue values in direct log submission
andrewlock Jan 6, 2026
b6e2f57
Add try-catch to log format
andrewlock Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions ...AutoInstrumentation/Logging/Serilog/DirectSubmission/Formatting/ILogEventPropertyValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// <copyright file="ILogEventPropertyValue.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable
using System;
using System.IO;

namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.Serilog.DirectSubmission.Formatting;

/// <summary>
/// Duck type for LogEventPropertyValue
/// https://github.com/serilog/serilog/blob/5e93d5045585095ebcb71ef340d6accd61f01670/src/Serilog/Events/LogEventPropertyValue.cs
/// </summary>
internal interface ILogEventPropertyValue
{
void Render(TextWriter output, string? format, IFormatProvider? formatProvider);
}
14 changes: 14 additions & 0 deletions ...er/AutoInstrumentation/Logging/Serilog/DirectSubmission/Formatting/SerilogLogFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
#nullable enable

using System.Collections;
using System.IO;
using System.Text;
using Datadog.Trace.DuckTyping;
using Datadog.Trace.Logging;
using Datadog.Trace.Logging.DirectSubmission;
using Datadog.Trace.Logging.DirectSubmission.Formatting;
using Datadog.Trace.Util;
using Datadog.Trace.Vendors.Newtonsoft.Json;
using Datadog.Trace.Vendors.Serilog.Events;

Expand Down Expand Up @@ -107,6 +109,18 @@ private static void FormatLogEventPropertyValue(JsonTextWriter writer, object va
return;
}

if (value.TryDuckCast<ILogEventPropertyValue>(out var propertyValue))
{
// This is likely an edge case, so we just write the value as a string and then embed that in the JSON
var sb = StringBuilderCache.Acquire();
var stringWriter = new StringWriter(sb);
propertyValue.Render(stringWriter, null, null);
Copy link
Collaborator

@NachoEchevarria NachoEchevarria Jan 7, 2026

Choose a reason for hiding this comment

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

If Render() launches an exception, we should probably catch it, right?

Copy link
Member Author

@andrewlock andrewlock Jan 7, 2026

Choose a reason for hiding this comment

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

That's actually an interesting one - currently we don't catch exceptions anywhere in here they bubble up and will cause a whole batch to be dropped. It seems like it would make sense to catch exceptions at the log level? 🤔 i.e. if we get an exception anywhere in the serialization, we should drop that log, but not the whole batch. Does that make sense to you?

Copy link
Member Author

@andrewlock andrewlock Jan 8, 2026

Choose a reason for hiding this comment

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

I added a try-catch at a higher level (where we call the top-level Format method, inside the direct log submission sink. That way we will catch any errors coming from formatter methods (there are multiple implementations, so I think it's better to be high-level here). That single log will then be dropped, but the overall batch isn't which seems preferable

var stringValue = stringWriter.ToString();
StringBuilderCache.Release(sb);
LogFormatter.WriteValue(writer, stringValue);
return;
}

// Always write null for unknown types to maintain valid JSON
LogFormatter.WriteValue(writer, value: null);

Expand Down
10 changes: 9 additions & 1 deletion tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/DirectSubmissionLogSink.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,15 @@ protected override async Task<bool> EmitBatch(Queue<DirectSubmissionLogEvent> ev
_logStringBuilder.Capacity = InitialBuilderSizeBytes;
}

log.Format(_logStringBuilder, _formatter);
try
{
log.Format(_logStringBuilder, _formatter);
}
catch (Exception ex)
{
_logger.Error(ex, "Log dropped due to an error during serialization");
continue;
}

// would be nice to avoid this, but can't see a way without direct UTF8 serialization (System.Text.Json)
// Currently a rogue giant message still pays the serialization cost but is then thrown away
Expand Down
Loading