Skip to content

Minor updates for libdatadog data pipeline changes #7115

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
merged 4 commits into from
Jun 20, 2025
+46 −61
Merged

Minor updates for libdatadog data pipeline changes #7115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fce54d8
Refactor SerilogLogger to expose configuration used to build the logger
andrewlock Jun 10, 2025
5b9d23f
Try fix and simplify copying of native libdatadog
andrewlock Jun 11, 2025
9ff3268
Fix build issue
andrewlock Jun 10, 2025
758cad7
Fix paths
andrewlock Jun 17, 2025
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
52 changes: 24 additions & 28 deletions tracer/build/_build/Build.Steps.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,44 +652,40 @@ async Task DownloadWafVersion(string libddwafVersion = null, string uncompressFo

Target CopyNativeFilesForTests => _ => _
.Unlisted()
.After(Clean)
.After(BuildTracerHome)
.After(Clean, BuildTracerHome)
.Before(RunIntegrationTests, RunManagedUnitTests)
.Executes(() =>
{
foreach(var projectName in Projects.NativeFilesDependentTests)
// Copy the native files to all the test projects for simplicity.
var testProjects = Solution.GetProjects("*Tests")
.Where(p => p.SolutionFolder.Name == "test"
&& p.Path.ToString().EndsWith(".csproj")); // exclude native test projects
foreach(var projectName in testProjects)
{
Logger.Information("Copying native files for project {ProjectName}", projectName);
var project = Solution.GetProject(projectName);
var testDir = project.Directory;
var testDir = project!.Directory;
var frameworks = project.GetTargetFrameworks();
var testBinFolder = testDir / "bin" / BuildConfiguration;

if (IsWin)
if (Framework is not null)
{
foreach (var framework in frameworks)
{
var source = MonitoringHomeDirectory / $"win-{TargetPlatform}" / "datadog_profiling_ffi.dll";
var dest = testBinFolder / framework / "LibDatadog.dll";
CopyFile(source, dest, FileExistsPolicy.Overwrite);
}
frameworks = frameworks.Where(x=> x == Framework).ToList();
}
else if (IsLinux)

var testBinFolder = testDir / "bin" / BuildConfiguration;

var (ext, source) = Platform switch
{
var (arch, ext) = GetUnixArchitectureAndExtension();
var source = MonitoringHomeDirectory / arch / $"libdatadog_profiling.{ext}";
foreach (var framework in frameworks)
{
var dest = testBinFolder / framework / $"LibDatadog.{ext}";
CopyFile(source, dest, FileExistsPolicy.Overwrite);
}
}
else if (IsOsx)
PlatformFamily.Windows => ("dll", MonitoringHomeDirectory / $"win-{TargetPlatform}" / "datadog_profiling_ffi.dll"),
PlatformFamily.Linux => ("so", MonitoringHomeDirectory / GetUnixArchitectureAndExtension().Arch / "libdatadog_profiling.so"),
PlatformFamily.OSX => ("dylib", MonitoringHomeDirectory / "osx" / $"libdatadog_profiling.dylib"),
_ => throw new NotSupportedException($"Unsupported platform: {Platform}")
};

foreach (var framework in frameworks)
{
var source = MonitoringHomeDirectory/ "osx" / $"libdatadog_profiling.dylib";
foreach (var framework in frameworks)
{
var dest = testBinFolder / framework / $"LibDatadog.dylib";
CopyFile(source, dest, FileExistsPolicy.Overwrite);
}
var dest = testBinFolder / framework / $"LibDatadog.{ext}";
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the approach but I have a question.

As we are looping through frameworks twice (once frameworks.Where(x=> x == Framework) and another time with this foreach) and we aren't using it between L673 and L684,
Wouldn't it be simpler to move the where predicate condition (x == Framework that is) and inverse it:

Suggested change
var dest = testBinFolder / framework / $"LibDatadog.{ext}";
if (framework != Framework)
{
continue;
}
var dest = testBinFolder / framework / $"LibDatadog.{ext}";

now maybe there is something I'm missing 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we could, but we would also need to add the Framework is not null case in there, so I think it's prob simpler as is tbh.

Copy link
Contributor

@bricefriha bricefriha Jun 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

true I missed that sorry.
Well in this case maybe we could simply do that 😁:

Suggested change
var dest = testBinFolder / framework / $"LibDatadog.{ext}";
if (Framework is not null && framework != Framework)
{
continue;
}
var dest = testBinFolder / framework / $"LibDatadog.{ext}";

therefore if Framework is null, we use the element as is. otherwise we compare it with Framework's value and skip if they are different.

I don't think it's more complicated.

I know I don't always suggest the prettiest code 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

One other thing is that the current code reads as something such as "get the frameworks that we care about, then process them" whereas the more optimized one reads as "process all of the frameworks, then determine whether we care about the framework"

It's really just a readability thing IMO, it may be more performant but I don't think that matters too much for this code in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do see your point, here.
I personally read the latter as "Go through all the frameworks and only process the one we care about".
But if you all find it more difficult to read, go for the former as It's for the build so I think readability is more important still

CopyFile(source, dest, FileExistsPolicy.Overwrite);
}
}
});
Expand Down
8 changes: 0 additions & 8 deletions tracer/build/_build/Projects.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ public static class Projects
public const string DebuggerUnreferencedExternal = "Samples.Probes.Unreferenced.External";

public const string RazorPages = "Samples.AspNetCoreRazorPages";

public static readonly string[] NativeFilesDependentTests = {
AppSecUnitTests,
ClrProfilerManagedTests,
TraceIntegrationTests,
TraceTests,
DdTraceIntegrationTests
};
}

public static class FileNames
Expand Down
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace.Coverage.collector/DataCollectorLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public DataCollectorLogger(DataCollectionLogger logger, DataCollectionContext co
fileSizeLimitBytes: fileConfig.MaxLogFileSizeBytes,
shared: true);

_datadogLogger = new DatadogSerilogLogger(loggerConfiguration.CreateLogger(), new NullLogRateLimiter(), fileConfig.LogDirectory);
_datadogLogger = new DatadogSerilogLogger(loggerConfiguration.CreateLogger(), new NullLogRateLimiter(), fileConfig);
}
}

Expand Down
15 changes: 5 additions & 10 deletions tracer/src/Datadog.Trace/Logging/Internal/Configuration/FileLoggingConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,11 @@
#nullable enable
namespace Datadog.Trace.Logging.Internal.Configuration;

internal readonly struct FileLoggingConfiguration
internal class FileLoggingConfiguration(long maxLogFileSizeBytes, string logDirectory, int logFileRetentionDays)
{
public readonly long MaxLogFileSizeBytes;
public readonly string LogDirectory;
public readonly int LogFileRetentionDays;
public long MaxLogFileSizeBytes { get; } = maxLogFileSizeBytes;

public FileLoggingConfiguration(long maxLogFileSizeBytes, string logDirectory, int logFileRetentionDays)
{
MaxLogFileSizeBytes = maxLogFileSizeBytes;
LogDirectory = logDirectory;
LogFileRetentionDays = logFileRetentionDays;
}
public string LogDirectory { get; } = logDirectory;

public int LogFileRetentionDays { get; } = logFileRetentionDays;
}
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/Logging/Internal/DatadogLoggingFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ static bool Contains(string?[]? array, string toMatch)
rateLimiter = new NullLogRateLimiter();
}

return new DatadogSerilogLogger(internalLogger, rateLimiter, config.File?.LogDirectory);
return new DatadogSerilogLogger(internalLogger, rateLimiter, config.File);
}

// Internal for testing
Expand Down
7 changes: 4 additions & 3 deletions tracer/src/Datadog.Trace/Logging/Internal/DatadogSerilogLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System;
using System.Runtime.CompilerServices;
using System.Threading;
using Datadog.Trace.Logging.Internal.Configuration;
using Datadog.Trace.Telemetry;
using Datadog.Trace.Telemetry.Metrics;
using Datadog.Trace.Vendors.Serilog;
Expand All @@ -24,16 +25,16 @@ internal class DatadogSerilogLogger : IDatadogLogger
private readonly ILogRateLimiter _rateLimiter;
private ILogger _logger;

public DatadogSerilogLogger(ILogger logger, ILogRateLimiter rateLimiter, string? fileLogDirectory)
public DatadogSerilogLogger(ILogger logger, ILogRateLimiter rateLimiter, FileLoggingConfiguration? fileLoggingConfiguration)
{
_logger = logger;
_rateLimiter = rateLimiter;
FileLogDirectory = fileLogDirectory;
FileLoggingConfiguration = fileLoggingConfiguration;
}

public static DatadogSerilogLogger NullLogger { get; } = new(SilentLogger.Instance, new NullLogRateLimiter(), null);

public string? FileLogDirectory { get; }
public FileLoggingConfiguration? FileLoggingConfiguration { get; }

public bool IsEnabled(LogEventLevel level) => _logger.IsEnabled(level);

Expand Down
3 changes: 2 additions & 1 deletion tracer/src/Datadog.Trace/Logging/Internal/IDatadogLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

using System;
using System.Runtime.CompilerServices;
using Datadog.Trace.Logging.Internal.Configuration;
using Datadog.Trace.Vendors.Serilog.Events;

namespace Datadog.Trace.Logging
{
internal interface IDatadogLogger
{
public string? FileLogDirectory { get; }
public FileLoggingConfiguration? FileLoggingConfiguration { get; }

bool IsEnabled(LogEventLevel level);

Expand Down
4 changes: 2 additions & 2 deletions tracer/src/Datadog.Trace/Logging/TracerFlare/TracerFlareManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private async Task<ApplyDetails[]> HandleTracerFlareInitiated(List<RemoteConfigu
Log.Debug(TracerFlareInitializationLog);

// dump the telemetry (assuming we have somewhere to dump it)
if (Log.FileLogDirectory is { } logDir)
if (Log.FileLoggingConfiguration?.LogDirectory is { } logDir)
{
// the filename here is chosen so that it will get cleaned up in the normal log rotation
ProcessHelpers.GetCurrentProcessInformation(out _, out _, out var pid);
Expand Down Expand Up @@ -226,7 +226,7 @@ private async Task<ApplyDetails[]> HandleTracerFlareRequested(List<RemoteConfigu
return AcknowledgeAll(config);
}

if (Log.FileLogDirectory is not { } fileLogDirectory)
if (Log.FileLoggingConfiguration?.LogDirectory is not { } fileLogDirectory)
{
Log.Debug("Ignoring tracer flare request - file logging is disabled");
return AcknowledgeAll(config);
Expand Down
14 changes: 7 additions & 7 deletions tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void UsesLogDirectoryWhenItExists(string obsoleteLogDirectory)
});

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeTrue();
config.File.Should().NotBeNull();
config.File?.LogDirectory.Should().Be(logDirectory);
}

Expand All @@ -61,7 +61,7 @@ public void UsesObsoleteLogDirectoryWhenAvailable(string logDirectory)
});

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeTrue();
config.File.Should().NotBeNull();
config.File?.LogDirectory.Should().Be(obsoleteLogDirectory);
}

Expand All @@ -82,7 +82,7 @@ public void UsesEnvironmentFallBackWhenBothNull(string logDirectory, string obso
});

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeTrue();
config.File.Should().NotBeNull();
config.File?.LogDirectory.Should().NotBeNullOrWhiteSpace();
}

Expand All @@ -95,7 +95,7 @@ public void CreatesLogDirectoryWhenItDoesntExist()
var source = new NameValueConfigurationSource(new() { { ConfigurationKeys.LogDirectory, logDirectory } });

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeTrue();
config.File.Should().NotBeNull();
config.File?.LogDirectory.Should().Be(logDirectory);
Directory.Exists(logDirectory).Should().BeTrue();
}
Expand Down Expand Up @@ -143,7 +143,7 @@ public void WhenNoSinksProvided_UsesFileSink()
var source = new NameValueConfigurationSource(new());

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeTrue();
config.File.Should().NotBeNull();
}

[Theory]
Expand All @@ -156,7 +156,7 @@ public void WhenFileSinkIsIncluded_UsesFileSink(string sinks)
var source = new NameValueConfigurationSource(new() { { ConfigurationKeys.LogSinks, sinks } });

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeTrue();
config.File.Should().NotBeNull();
}

[Theory]
Expand All @@ -169,7 +169,7 @@ public void WhenFileSinkIsNotIncluded_DoesNotUseFileSink(string sinks)
var source = new NameValueConfigurationSource(new() { { ConfigurationKeys.LogSinks, sinks } });

var config = DatadogLoggingFactory.GetConfiguration(source, NullConfigurationTelemetry.Instance);
config.File.HasValue.Should().BeFalse();
config.File.Should().BeNull();
}
}
}
Loading