Skip to content

Commit

Permalink
[CI Visibility] Set TracerSettings using a configuration source ins…
Browse files Browse the repository at this point in the history
…tead of mutating (#6399)

## Summary of changes

Change how CI Visibility creates its `TracerSettings` object to avoid
mutating settings afterwards

## Reason for change

This stack of changes is about removing duplication (among other things)
in the `TracerSettings` etc related objects. This is _partially_
required in order to remove the "snapshot generator" complexity that was
removed in #6370. Given
`TracerSettings` are not exposed publicly in Datadog.Trace, we want to
move to doing a "one shot" configuring of them, ultimately so that we
can make the object immutable (and remove `ImmutableTracerSettings` and
friends).

## Implementation details

CI Visibility is one of the few places where we mutate settings _after_
creating the `TracerSettings`. This is mostly because there's additional
logic that CI Visibility wants to perform.

Ultimately, the "solution" to that issue in this PR is to move that
logic into the `TracerSettings` constructor. I'm not entirely happy
about it, but it's the only approach I could find that works.

- Add a "dummy" configuration value to a configuration source when
creating the `TracerSettings`. This is used purely as a "switch" to say
"we're in CI Visibility mode".
- We absolutely could just pass this in as a constructor parameter. I
went that route first and then backed away, but can totally be swayed.
- Add an additional configuration source to update settings that we want
to change in CI Vis (e.g. logs injection).
- In the `TracerSettings` ctor, add an additional ignored URL when in CI
Vis mode, modify the default service name (if required) and add an
additional GlobalTag.

## Test coverage

I'd love to have some, but the CI Visibility configuration is kinda all
over the place, so if you have any pointers @tonyredondo I'm all ears...

## Other details

Part of stack 
- #6370
- #6376
- #6385
- #6386
- #6397 
- #6399 👈 This PR
- #6400  
- #6405
- #6408
  • Loading branch information
andrewlock authored and veerbia committed Dec 16, 2024
1 parent da150c9 commit 8cec0e1
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 52 deletions.
30 changes: 0 additions & 30 deletions tracer/src/Datadog.Trace/Ci/CIVisibility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,6 @@ public static void Initialize()

var tracerSettings = settings.TracerSettings;
Log.Debug("Setting up the test session name to: {TestSessionName}", settings.TestSessionName);

// Set the service name if empty
if (string.IsNullOrEmpty(tracerSettings.ServiceName))
{
// Extract repository name from the git url and use it as a default service name.
tracerSettings.ServiceName = GetServiceNameFromRepository(CIEnvironmentValues.Instance.Repository);
tracerSettings.GlobalTags[CommonTags.UserProvidedTestServiceTag] = "false";
}
else
{
tracerSettings.GlobalTags[CommonTags.UserProvidedTestServiceTag] = "true";
}

// Normalize the service name
tracerSettings.ServiceName = NormalizerTraceProcessor.NormalizeService(tracerSettings.ServiceName);
Log.Debug("Setting up the service name to: {ServiceName}", tracerSettings.ServiceName);

// Initialize Tracer
Expand Down Expand Up @@ -203,21 +188,6 @@ internal static void InitializeFromRunner(CIVisibilitySettings settings, IDiscov

var tracerSettings = settings.TracerSettings;
Log.Debug("Setting up the test session name to: {TestSessionName}", settings.TestSessionName);

// Set the service name if empty
if (string.IsNullOrEmpty(tracerSettings.ServiceName))
{
// Extract repository name from the git url and use it as a default service name.
tracerSettings.ServiceName = GetServiceNameFromRepository(CIEnvironmentValues.Instance.Repository);
tracerSettings.GlobalTags[CommonTags.UserProvidedTestServiceTag] = "false";
}
else
{
tracerSettings.GlobalTags[CommonTags.UserProvidedTestServiceTag] = "true";
}

// Normalize the service name
tracerSettings.ServiceName = NormalizerTraceProcessor.NormalizeService(tracerSettings.ServiceName);
Log.Debug("Setting up the service name to: {ServiceName}", tracerSettings.ServiceName);

// Initialize Tracer
Expand Down
39 changes: 19 additions & 20 deletions tracer/src/Datadog.Trace/Ci/Configuration/CIVisibilitySettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#nullable enable

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -298,32 +299,30 @@ internal void SetDefaultManualInstrumentationSettings()
}

private TracerSettings InitializeTracerSettings()
{
var source = GlobalConfigurationSource.CreateDefaultConfigurationSource();
var defaultExcludedUrlSubstrings = string.Empty;
var configResult = source.GetString(ConfigurationKeys.HttpClientExcludedUrlSubstrings, NullConfigurationTelemetry.Instance, validator: null, recordValue: false);
if (configResult is { IsValid: true, Result: { } substrings } && !string.IsNullOrWhiteSpace(substrings))
{
defaultExcludedUrlSubstrings = substrings + ", ";
}
=> InitializeTracerSettings(GlobalConfigurationSource.CreateDefaultConfigurationSource());

source.Insert(0, new NameValueConfigurationSource(
new NameValueCollection
{
[ConfigurationKeys.HttpClientExcludedUrlSubstrings] = defaultExcludedUrlSubstrings + "/session/FakeSessionIdForPollingPurposes",
},
ConfigurationOrigins.Calculated));

var tracerSettings = new TracerSettings(source, new ConfigurationTelemetry(), new OverrideErrorLog());
// Internal for testing
internal TracerSettings InitializeTracerSettings(CompositeConfigurationSource source)
{
// This is a somewhat hacky way to "tell" TracerSettings that we're running in CI Visibility
// There's no doubt various other ways we could flag it based on values we're _already_ extracting,
// but we don't want to set up too much interdependence there.
var telemetry = new ConfigurationTelemetry();
var additionalSource = new Dictionary<string, object?> { { ConfigurationKeys.CIVisibility.IsRunningInCiVisMode, true } };

if (Logs)
{
// Enable the direct log submission
tracerSettings.LogSubmissionSettings.DirectLogSubmissionEnabledIntegrations.Add("XUnit");
tracerSettings.LogSubmissionSettings.DirectLogSubmissionBatchPeriod = TimeSpan.FromSeconds(1);
// fetch the "original" values, and update them with the CI Visibility settings
var enabledDirectLogSubmissionIntegrations = new ConfigurationBuilder(source, telemetry)
.WithKeys(ConfigurationKeys.DirectLogSubmission.EnabledIntegrations)
.AsString();
var logIntegrations = enabledDirectLogSubmissionIntegrations + ";XUnit";
additionalSource[ConfigurationKeys.DirectLogSubmission.EnabledIntegrations] = logIntegrations;
additionalSource[ConfigurationKeys.DirectLogSubmission.BatchPeriodSeconds] = 1;
}

return tracerSettings;
var newSource = new CompositeConfigurationSource([new DictionaryObjectConfigurationSource(additionalSource), source]);
return new TracerSettings(newSource, telemetry, new OverrideErrorLog());
}
}
}
5 changes: 5 additions & 0 deletions tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.cs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ internal static partial class ConfigurationKeys
/// </summary>
public static class CIVisibility
{
/// <summary>
/// An internal key used to "tell" tracer settings that we're running in CI Visibility mode
/// </summary>
public const string IsRunningInCiVisMode = "_DD_INTERNAL_IS_RUNNING_IN_CIVISIBILITY";

/// <summary>
/// Configuration key for enabling or disabling CI Visibility.
/// Default value is false (disabled).
Expand Down
50 changes: 48 additions & 2 deletions tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text.RegularExpressions;
using Datadog.Trace.Ci;
using Datadog.Trace.Ci.CiEnvironment;
using Datadog.Trace.ClrProfiler;
using Datadog.Trace.ClrProfiler.ServerlessInstrumentation;
using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
using Datadog.Trace.Configuration.Telemetry;
using Datadog.Trace.Logging;
using Datadog.Trace.Logging.DirectSubmission;
using Datadog.Trace.Processors;
using Datadog.Trace.Propagators;
using Datadog.Trace.Sampling;
using Datadog.Trace.SourceGenerators;
Expand Down Expand Up @@ -87,6 +90,11 @@ internal TracerSettings(IConfigurationSource? source, IConfigurationTelemetry te
GCPFunctionSettings = new ImmutableGCPFunctionSettings(source, _telemetry);
IsRunningInGCPFunctions = GCPFunctionSettings.IsGCPFunction;

// We don't want/need to record this value, so explicitly use null telemetry
var isRunningInCiVisibility = new ConfigurationBuilder(source, NullConfigurationTelemetry.Instance)
.WithKeys(ConfigurationKeys.CIVisibility.IsRunningInCiVisMode)
.AsBool(false);

LambdaMetadata = LambdaMetadata.Create();

IsRunningInAzureAppService = ImmutableAzureAppServiceSettings.GetIsAzureAppService(source, telemetry);
Expand Down Expand Up @@ -124,11 +132,34 @@ _ when x.ToBoolean() is { } boolean => boolean,
.AsString();

var otelServiceName = config.WithKeys(ConfigurationKeys.OpenTelemetry.ServiceName).AsStringResult();
ServiceName = config
var serviceName = config
.WithKeys(ConfigurationKeys.ServiceName, "DD_SERVICE_NAME")
.AsStringResult()
.OverrideWith(in otelServiceName, ErrorLog);

var isUserProvidedTestServiceTag = true;
if (isRunningInCiVisibility)
{
// Set the service name if not set
var ciVisServiceName = serviceName;
if (string.IsNullOrEmpty(serviceName))
{
// Extract repository name from the git url and use it as a default service name.
ciVisServiceName = CIVisibility.GetServiceNameFromRepository(CIEnvironmentValues.Instance.Repository);
isUserProvidedTestServiceTag = false;
}

// Normalize the service name
ciVisServiceName = NormalizerTraceProcessor.NormalizeService(ciVisServiceName);
if (ciVisServiceName != serviceName)
{
serviceName = ciVisServiceName;
telemetry.Record(ConfigurationKeys.ServiceName, serviceName, recordValue: true, ConfigurationOrigins.Calculated);
}
}

ServiceName = serviceName;

ServiceVersion = config
.WithKeys(ConfigurationKeys.ServiceVersion)
.AsString();
Expand Down Expand Up @@ -204,6 +235,11 @@ _ when x.ToBoolean() is { } boolean => boolean,
.Where(kvp => !string.IsNullOrWhiteSpace(kvp.Key) && !string.IsNullOrWhiteSpace(kvp.Value))
.ToDictionary(kvp => kvp.Key.Trim(), kvp => kvp.Value.Trim());

if (isRunningInCiVisibility)
{
GlobalTags[Ci.Tags.CommonTags.UserProvidedTestServiceTag] = isUserProvidedTestServiceTag ? "true" : "false";
}

var headerTagsNormalizationFixEnabled = config
.WithKeys(ConfigurationKeys.FeatureFlags.HeaderTagsNormalizationFixEnabled)
.AsBool(defaultValue: true);
Expand Down Expand Up @@ -481,9 +517,19 @@ _ when x.ToBoolean() is { } boolean => boolean,
IsRunningInAzureAppService ? ImmutableAzureAppServiceSettings.DefaultHttpClientExclusions :
LambdaMetadata is { IsRunningInLambda: true } m ? m.DefaultHttpClientExclusions : string.Empty);

if (isRunningInCiVisibility)
{
// always add the additional exclude in ci vis
const string fakeSessionEndpoint = "/session/FakeSessionIdForPollingPurposes";
urlSubstringSkips = string.IsNullOrEmpty(urlSubstringSkips)
? fakeSessionEndpoint
: $"{urlSubstringSkips},{fakeSessionEndpoint}";
telemetry.Record(ConfigurationKeys.HttpClientExcludedUrlSubstrings, urlSubstringSkips, recordValue: true, ConfigurationOrigins.Calculated);
}

HttpClientExcludedUrlSubstrings = !string.IsNullOrEmpty(urlSubstringSkips)
? TrimSplitString(urlSubstringSkips.ToUpperInvariant(), commaSeparator)
: Array.Empty<string>();
: [];

DbmPropagationMode = config
.WithKeys(ConfigurationKeys.DbmPropagationMode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class ConfigurationTests
// Lambda handler extracts these directly from env var, and no reason to think that will change
"_DD_EXTENSION_ENDPOINT",
"_DD_EXTENSION_PATH",
// Internal variable just used to "pass" settings to the
"_DD_INTERNAL_IS_RUNNING_IN_CIVISIBILITY",
// mini agent uses this directly from env var, and no reason to think that will change
"DD_MINI_AGENT_PATH",
"DD_ENTITY_ID", // Datadog.Trace.Vendors.StatsdClient.StatsdConfig.EntityIdEnvVar (we don't use this, it was just vendored in)
Expand Down

0 comments on commit 8cec0e1

Please sign in to comment.