From 066250eb93c2be5c6fb0e377ff00eeac2d02277a Mon Sep 17 00:00:00 2001 From: Tony Redondo Date: Fri, 4 Oct 2024 00:56:19 +0200 Subject: [PATCH] internal/civisibility: automatic test retries (#2892) --- internal/civisibility/constants/env.go | 10 + internal/civisibility/constants/test_tags.go | 4 + .../civisibility/integrations/civisibility.go | 9 +- .../integrations/civisibility_features.go | 113 ++++++++ .../integrations/gotesting/instrumentation.go | 251 ++++++++++++++++-- .../integrations/gotesting/reflections.go | 196 ++++++++++++-- .../integrations/gotesting/testing.go | 58 +++- .../integrations/gotesting/testingB.go | 2 +- .../integrations/gotesting/testingT.go | 2 +- .../integrations/gotesting/testing_test.go | 132 +++++++-- internal/civisibility/utils/names.go | 11 + internal/civisibility/utils/net/client.go | 24 +- 12 files changed, 723 insertions(+), 89 deletions(-) create mode 100644 internal/civisibility/integrations/civisibility_features.go diff --git a/internal/civisibility/constants/env.go b/internal/civisibility/constants/env.go index 2de4abb39d..66e6de7636 100644 --- a/internal/civisibility/constants/env.go +++ b/internal/civisibility/constants/env.go @@ -27,4 +27,14 @@ const ( // CIVisibilityTestSessionNameEnvironmentVariable indicate the test session name to be used on CI Visibility payloads CIVisibilityTestSessionNameEnvironmentVariable = "DD_TEST_SESSION_NAME" + + // CIVisibilityFlakyRetryEnabledEnvironmentVariable kill-switch that allows to explicitly disable retries even if the remote setting is enabled. + // This environment variable should be set to "0" or "false" to disable the flaky retry feature. + CIVisibilityFlakyRetryEnabledEnvironmentVariable = "DD_CIVISIBILITY_FLAKY_RETRY_ENABLED" + + // CIVisibilityFlakyRetryCountEnvironmentVariable indicates the maximum number of retry attempts for a single test case. + CIVisibilityFlakyRetryCountEnvironmentVariable = "DD_CIVISIBILITY_FLAKY_RETRY_COUNT" + + // CIVisibilityTotalFlakyRetryCountEnvironmentVariable indicates the maximum number of retry attempts for the entire session. + CIVisibilityTotalFlakyRetryCountEnvironmentVariable = "DD_CIVISIBILITY_TOTAL_FLAKY_RETRY_COUNT" ) diff --git a/internal/civisibility/constants/test_tags.go b/internal/civisibility/constants/test_tags.go index bb49fe696d..443f635c8a 100644 --- a/internal/civisibility/constants/test_tags.go +++ b/internal/civisibility/constants/test_tags.go @@ -65,6 +65,10 @@ const ( // TestSessionName indicates the test session name // This constant is used to tag traces with the test session name TestSessionName = "test_session.name" + + // TestIsRetry indicates a retry execution + // This constant is used to tag test events that are part of a retry execution + TestIsRetry = "test.is_retry" ) // Define valid test status types. diff --git a/internal/civisibility/integrations/civisibility.go b/internal/civisibility/integrations/civisibility.go index 6ce993b8e3..92c0e2ee77 100644 --- a/internal/civisibility/integrations/civisibility.go +++ b/internal/civisibility/integrations/civisibility.go @@ -70,7 +70,8 @@ func internalCiVisibilityInitialization(tracerInitializer func([]tracer.StartOpt // Check if DD_SERVICE has been set; otherwise default to the repo name (from the spec). var opts []tracer.StartOption - if v := os.Getenv("DD_SERVICE"); v == "" { + serviceName := os.Getenv("DD_SERVICE") + if serviceName == "" { if repoURL, ok := ciTags[constants.GitRepositoryURL]; ok { // regex to sanitize the repository url to be used as a service name repoRegex := regexp.MustCompile(`(?m)/([a-zA-Z0-9\-_.]*)$`) @@ -78,10 +79,14 @@ func internalCiVisibilityInitialization(tracerInitializer func([]tracer.StartOpt if len(matches) > 1 { repoURL = strings.TrimSuffix(matches[1], ".git") } - opts = append(opts, tracer.WithService(repoURL)) + serviceName = repoURL + opts = append(opts, tracer.WithService(serviceName)) } } + // Initializing additional features asynchronously + go func() { ensureAdditionalFeaturesInitialization(serviceName) }() + // Initialize the tracer tracerInitializer(opts) diff --git a/internal/civisibility/integrations/civisibility_features.go b/internal/civisibility/integrations/civisibility_features.go new file mode 100644 index 0000000000..c12d5cc0f1 --- /dev/null +++ b/internal/civisibility/integrations/civisibility_features.go @@ -0,0 +1,113 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024 Datadog, Inc. + +package integrations + +import ( + "sync" + + "gopkg.in/DataDog/dd-trace-go.v1/internal" + "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/constants" + "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/utils/net" + "gopkg.in/DataDog/dd-trace-go.v1/internal/log" +) + +const ( + DefaultFlakyRetryCount = 5 + DefaultFlakyTotalRetryCount = 1_000 +) + +type ( + // FlakyRetriesSetting struct to hold all the settings related to flaky tests retries + FlakyRetriesSetting struct { + RetryCount int64 + TotalRetryCount int64 + RemainingTotalRetryCount int64 + } +) + +var ( + // additionalFeaturesInitializationOnce ensures we do the additional features initialization just once + additionalFeaturesInitializationOnce sync.Once + + // ciVisibilityRapidClient contains the http rapid client to do CI Visibility queries and upload to the rapid backend + ciVisibilityClient net.Client + + // ciVisibilitySettings contains the CI Visibility settings for this session + ciVisibilitySettings net.SettingsResponseData + + // ciVisibilityEarlyFlakyDetectionSettings contains the CI Visibility Early Flake Detection data for this session + ciVisibilityEarlyFlakyDetectionSettings net.EfdResponseData + + // ciVisibilityFlakyRetriesSettings contains the CI Visibility Flaky Retries settings for this session + ciVisibilityFlakyRetriesSettings FlakyRetriesSetting +) + +// ensureAdditionalFeaturesInitialization initialize all the additional features +func ensureAdditionalFeaturesInitialization(serviceName string) { + additionalFeaturesInitializationOnce.Do(func() { + // Create the CI Visibility client + ciVisibilityClient = net.NewClientWithServiceName(serviceName) + if ciVisibilityClient == nil { + log.Error("CI Visibility: error getting the ci visibility http client") + return + } + + // Get the CI Visibility settings payload for this test session + ciSettings, err := ciVisibilityClient.GetSettings() + if err != nil { + log.Error("CI Visibility: error getting CI visibility settings: %v", err) + } else if ciSettings != nil { + ciVisibilitySettings = *ciSettings + } + + // if early flake detection is enabled then we run the early flake detection request + if ciVisibilitySettings.EarlyFlakeDetection.Enabled { + ciEfdData, err := ciVisibilityClient.GetEarlyFlakeDetectionData() + if err != nil { + log.Error("CI Visibility: error getting CI visibility early flake detection data: %v", err) + } else if ciEfdData != nil { + ciVisibilityEarlyFlakyDetectionSettings = *ciEfdData + } + } + + // if flaky test retries is enabled then let's load the flaky retries settings + if ciVisibilitySettings.FlakyTestRetriesEnabled { + flakyRetryEnabledByEnv := internal.BoolEnv(constants.CIVisibilityFlakyRetryEnabledEnvironmentVariable, true) + if flakyRetryEnabledByEnv { + totalRetriesCount := (int64)(internal.IntEnv(constants.CIVisibilityTotalFlakyRetryCountEnvironmentVariable, DefaultFlakyTotalRetryCount)) + ciVisibilityFlakyRetriesSettings = FlakyRetriesSetting{ + RetryCount: (int64)(internal.IntEnv(constants.CIVisibilityFlakyRetryCountEnvironmentVariable, DefaultFlakyRetryCount)), + TotalRetryCount: totalRetriesCount, + RemainingTotalRetryCount: totalRetriesCount, + } + } else { + log.Warn("CI Visibility: flaky test retries was disabled by the environment variable") + ciVisibilitySettings.FlakyTestRetriesEnabled = false + } + } + }) +} + +// GetSettings gets the settings from the backend settings endpoint +func GetSettings() *net.SettingsResponseData { + // call to ensure the additional features initialization is completed (service name can be null here) + ensureAdditionalFeaturesInitialization("") + return &ciVisibilitySettings +} + +// GetEarlyFlakeDetectionSettings gets the early flake detection known tests data +func GetEarlyFlakeDetectionSettings() *net.EfdResponseData { + // call to ensure the additional features initialization is completed (service name can be null here) + ensureAdditionalFeaturesInitialization("") + return &ciVisibilityEarlyFlakyDetectionSettings +} + +// GetFlakyRetriesSettings gets the flaky retries settings +func GetFlakyRetriesSettings() *FlakyRetriesSetting { + // call to ensure the additional features initialization is completed (service name can be null here) + ensureAdditionalFeaturesInitialization("") + return &ciVisibilityFlakyRetriesSettings +} diff --git a/internal/civisibility/integrations/gotesting/instrumentation.go b/internal/civisibility/integrations/gotesting/instrumentation.go index 5f1d2bf849..af40c23570 100644 --- a/internal/civisibility/integrations/gotesting/instrumentation.go +++ b/internal/civisibility/integrations/gotesting/instrumentation.go @@ -28,14 +28,20 @@ import ( // automatic instrumentation type ( + // instrumentationMetadata contains the internal instrumentation metadata instrumentationMetadata struct { IsInternal bool } - ddTestItem struct { - test integrations.DdTest - error atomic.Int32 - skipped atomic.Int32 + // testExecutionMetadata contains metadata regarding an unique *testing.T or *testing.B execution + testExecutionMetadata struct { + test integrations.DdTest // internal CI Visibility test event + error atomic.Int32 // flag to check if the test event has error data already + skipped atomic.Int32 // flag to check if the test event has skipped data already + panicData any // panic data recovered from an internal test execution when using an additional feature wrapper + panicStacktrace string // stacktrace from the panic recovered from an internal test + isARetry bool // flag to tag if a current test execution is a retry + hasAdditionalFeatureWrapper bool // flag to check if the current execution is part of an additional feature wrapper } ) @@ -49,11 +55,11 @@ var ( // instrumentationMapMutex is a read-write mutex for synchronizing access to instrumentationMap. instrumentationMapMutex sync.RWMutex - // ciVisibilityTests holds a map of *testing.T or *testing.B to civisibility.DdTest for tracking tests. - ciVisibilityTests = map[unsafe.Pointer]*ddTestItem{} + // ciVisibilityTests holds a map of *testing.T or *testing.B to execution metadata for tracking tests. + ciVisibilityTestMetadata = map[unsafe.Pointer]*testExecutionMetadata{} - // ciVisibilityTestsMutex is a read-write mutex for synchronizing access to ciVisibilityTests. - ciVisibilityTestsMutex sync.RWMutex + // ciVisibilityTestMetadataMutex is a read-write mutex for synchronizing access to ciVisibilityTestMetadata. + ciVisibilityTestMetadataMutex sync.RWMutex ) // isCiVisibilityEnabled gets if CI Visibility has been enabled or disabled by the "DD_CIVISIBILITY_ENABLED" environment variable @@ -95,21 +101,31 @@ func setInstrumentationMetadata(fn *runtime.Func, metadata *instrumentationMetad instrumentationMap[fn] = metadata } -// getCiVisibilityTest retrieves the CI visibility test associated with a given *testing.T, *testing.B, *testing.common -func getCiVisibilityTest(tb testing.TB) *ddTestItem { - ciVisibilityTestsMutex.RLock() - defer ciVisibilityTestsMutex.RUnlock() - if v, ok := ciVisibilityTests[reflect.ValueOf(tb).UnsafePointer()]; ok { +// createTestMetadata creates the CI visibility test metadata associated with a given *testing.T, *testing.B, *testing.common +func createTestMetadata(tb testing.TB) *testExecutionMetadata { + ciVisibilityTestMetadataMutex.RLock() + defer ciVisibilityTestMetadataMutex.RUnlock() + execMetadata := &testExecutionMetadata{} + ciVisibilityTestMetadata[reflect.ValueOf(tb).UnsafePointer()] = execMetadata + return execMetadata +} + +// getTestMetadata retrieves the CI visibility test metadata associated with a given *testing.T, *testing.B, *testing.common +func getTestMetadata(tb testing.TB) *testExecutionMetadata { + ciVisibilityTestMetadataMutex.RLock() + defer ciVisibilityTestMetadataMutex.RUnlock() + ptr := reflect.ValueOf(tb).UnsafePointer() + if v, ok := ciVisibilityTestMetadata[ptr]; ok { return v } return nil } -// setCiVisibilityTest associates a CI visibility test with a given *testing.T, *testing.B, *testing.common -func setCiVisibilityTest(tb testing.TB, ciTest integrations.DdTest) { - ciVisibilityTestsMutex.Lock() - defer ciVisibilityTestsMutex.Unlock() - ciVisibilityTests[reflect.ValueOf(tb).UnsafePointer()] = &ddTestItem{test: ciTest} +// deleteTestMetadata delete the CI visibility test metadata associated with a given *testing.T, *testing.B, *testing.common +func deleteTestMetadata(tb testing.TB) { + ciVisibilityTestMetadataMutex.RLock() + defer ciVisibilityTestMetadataMutex.RUnlock() + delete(ciVisibilityTestMetadata, reflect.ValueOf(tb).UnsafePointer()) } // instrumentTestingM helper function to instrument internalTests and internalBenchmarks in a `*testing.M` instance. @@ -196,14 +212,30 @@ func instrumentTestingTFunc(f func(*testing.T)) func(*testing.T) { suite := module.GetOrCreateSuite(suiteName) test := suite.CreateTest(t.Name()) test.SetTestFunc(originalFunc) - setCiVisibilityTest(t, test) + + // Get the metadata regarding the execution (in case is already created from the additional features) + execMeta := getTestMetadata(t) + if execMeta == nil { + // in case there's no additional features then we create the metadata for this execution and defer the disposal + execMeta = createTestMetadata(t) + defer deleteTestMetadata(t) + } + + // Set the CI visibility test. + execMeta.test = test + defer func() { if r := recover(); r != nil { // Handle panic and set error information. test.SetErrorInfo("panic", fmt.Sprint(r), utils.GetStacktrace(1)) test.Close(integrations.ResultStatusFail) checkModuleAndSuite(module, suite) - integrations.ExitCiVisibility() + // this is not an internal test. Retries are not applied to subtest (because the parent internal test is going to be retried) + // so for this case we avoid closing CI Visibility, but we don't stop the panic from happening. + // it will be handled by `t.Run` + if checkIfCIVisibilityExitIsRequiredByPanic() { + integrations.ExitCiVisibility() + } panic(r) } else { // Normal finalization: determine the test result based on its state. @@ -224,6 +256,7 @@ func instrumentTestingTFunc(f func(*testing.T)) func(*testing.T) { // Execute the original test function. f(t) } + setInstrumentationMetadata(runtime.FuncForPC(reflect.Indirect(reflect.ValueOf(instrumentedFn)).Pointer()), &instrumentationMetadata{IsInternal: true}) return instrumentedFn } @@ -236,8 +269,8 @@ func instrumentSetErrorInfo(tb testing.TB, errType string, errMessage string, sk } // Get the CI Visibility span and check if we can set the error type, message and stack - ciTestItem := getCiVisibilityTest(tb) - if ciTestItem != nil && ciTestItem.error.CompareAndSwap(0, 1) && ciTestItem.test != nil { + ciTestItem := getTestMetadata(tb) + if ciTestItem != nil && ciTestItem.test != nil && ciTestItem.error.CompareAndSwap(0, 1) { ciTestItem.test.SetErrorInfo(errType, errMessage, utils.GetStacktrace(2+skip)) } } @@ -250,8 +283,8 @@ func instrumentCloseAndSkip(tb testing.TB, skipReason string) { } // Get the CI Visibility span and check if we can mark it as skipped and close it - ciTestItem := getCiVisibilityTest(tb) - if ciTestItem != nil && ciTestItem.skipped.CompareAndSwap(0, 1) && ciTestItem.test != nil { + ciTestItem := getTestMetadata(tb) + if ciTestItem != nil && ciTestItem.test != nil && ciTestItem.skipped.CompareAndSwap(0, 1) { ciTestItem.test.CloseWithFinishTimeAndSkipReason(integrations.ResultStatusSkip, time.Now(), skipReason) } } @@ -264,8 +297,8 @@ func instrumentSkipNow(tb testing.TB) { } // Get the CI Visibility span and check if we can mark it as skipped and close it - ciTestItem := getCiVisibilityTest(tb) - if ciTestItem != nil && ciTestItem.skipped.CompareAndSwap(0, 1) && ciTestItem.test != nil { + ciTestItem := getTestMetadata(tb) + if ciTestItem != nil && ciTestItem.test != nil && ciTestItem.skipped.CompareAndSwap(0, 1) { ciTestItem.test.Close(integrations.ResultStatusSkip) } } @@ -347,8 +380,17 @@ func instrumentTestingBFunc(pb *testing.B, name string, f func(*testing.B)) (str iPfOfB = getBenchmarkPrivateFields(b) // Replace this function with the original one (executed only once - the first iteration[b.run1]). *iPfOfB.benchFunc = f - // Set b to the CI visibility test. - setCiVisibilityTest(b, test) + + // Get the metadata regarding the execution (in case is already created from the additional features) + execMeta := getTestMetadata(b) + if execMeta == nil { + // in case there's no additional features then we create the metadata for this execution and defer the disposal + execMeta = createTestMetadata(b) + defer deleteTestMetadata(b) + } + + // Set the CI visibility test. + execMeta.test = test // Enable the timer again. b.ResetTimer() @@ -419,3 +461,156 @@ func instrumentTestingBFunc(pb *testing.B, name string, f func(*testing.B)) (str setCiVisibilityBenchmarkFunc(runtime.FuncForPC(reflect.Indirect(reflect.ValueOf(instrumentedFunc)).Pointer())) return subBenchmarkAutoName, instrumentedFunc } + +// checkIfCIVisibilityExitIsRequiredByPanic checks the additional features settings to decide if we allow individual tests to panic or not +func checkIfCIVisibilityExitIsRequiredByPanic() bool { + // Apply additional features + settings := integrations.GetSettings() + + // If we don't plan to do retries then we allow to panic + return !settings.FlakyTestRetriesEnabled && !settings.EarlyFlakeDetection.Enabled +} + +// applyAdditionalFeaturesToTestFunc applies all the additional features as wrapper of a func(*testing.T) +func applyAdditionalFeaturesToTestFunc(f func(*testing.T)) func(*testing.T) { + // Apply additional features + settings := integrations.GetSettings() + + // Wrapper function + wrapperFunc := f + + // Flaky test retries + if settings.FlakyTestRetriesEnabled { + flakyRetrySettings := integrations.GetFlakyRetriesSettings() + + // if the retry count per test is > 1 and if we still have remaining total retry count + if flakyRetrySettings.RetryCount > 1 && flakyRetrySettings.RemainingTotalRetryCount > 0 { + wrapperFunc = func(t *testing.T) { + retryCount := flakyRetrySettings.RetryCount + executionIndex := -1 + var panicExecution *testExecutionMetadata + + // Get the private fields from the *testing.T instance + tParentCommonPrivates := getTestParentPrivateFields(t) + + // Module and suite for this test + var module integrations.DdTestModule + var suite integrations.DdTestSuite + + for { + // increment execution index + executionIndex++ + + // we need to create a new local copy of `t` as a way to isolate the results of this execution. + // this is because we don't want these executions to affect the overall result of the test process + // nor the parent test status. + ptrToLocalT := &testing.T{} + copyTestWithoutParent(t, ptrToLocalT) + + // we create a dummy parent so we can run the test using this local copy + // without affecting the test parent + localTPrivateFields := getTestPrivateFields(ptrToLocalT) + *localTPrivateFields.parent = unsafe.Pointer(&testing.T{}) + + // create an execution metadata instance + execMeta := createTestMetadata(ptrToLocalT) + execMeta.hasAdditionalFeatureWrapper = true + + // if we are in a retry execution we set the `isARetry` flag so we can tag the test event. + if executionIndex > 0 { + execMeta.isARetry = true + } + + // run original func similar to it gets run internally in tRunner + chn := make(chan struct{}, 1) + go func() { + defer func() { + chn <- struct{}{} + }() + f(ptrToLocalT) + }() + <-chn + + // we call the cleanup funcs after this execution before trying another execution + callTestCleanupPanicValue := testingTRunCleanup(ptrToLocalT, 1) + if callTestCleanupPanicValue != nil { + fmt.Printf("cleanup error: %v\n", callTestCleanupPanicValue) + } + + // extract module and suite if present + currentSuite := execMeta.test.Suite() + if suite == nil && currentSuite != nil { + suite = currentSuite + } + if module == nil && currentSuite != nil && currentSuite.Module() != nil { + module = currentSuite.Module() + } + + // remove execution metadata + deleteTestMetadata(ptrToLocalT) + + // decrement retry counts + remainingRetries := atomic.AddInt64(&retryCount, -1) + remainingTotalRetries := atomic.AddInt64(&flakyRetrySettings.RemainingTotalRetryCount, -1) + + // if a panic occurs we fail the test + if execMeta.panicData != nil { + ptrToLocalT.Fail() + + // stores the first panic data so we can do a panic later after all retries + if panicExecution == nil { + panicExecution = execMeta + } + } + + // if not failed and if there's no panic data then we don't do any retry + // if there's no more retries we also exit the loop + if !ptrToLocalT.Failed() || remainingRetries < 0 || remainingTotalRetries < 0 { + // because we are not going to do any other retry we set the original `t` with the results + // and in case of failure we mark the parent test as failed as well. + tCommonPrivates := getTestPrivateFields(t) + tCommonPrivates.SetFailed(ptrToLocalT.Failed()) + tCommonPrivates.SetSkipped(ptrToLocalT.Skipped()) + tParentCommonPrivates.SetFailed(ptrToLocalT.Failed()) + break + } + } + + // in case we execute some retries then let's print a summary of the result with the retries count + retries := flakyRetrySettings.RetryCount - (retryCount + 1) + if retries > 0 { + status := "passed" + if t.Failed() { + status = "failed" + } else if t.Skipped() { + status = "skipped" + } + + fmt.Printf(" [ %v after %v retries ]\n", status, retries) + } + + // after all test executions we check if we need to close the suite and the module + checkModuleAndSuite(module, suite) + + // let's check if total retry count was exceeded + if flakyRetrySettings.RemainingTotalRetryCount < 1 { + fmt.Println(" the maximum number of total retries was exceeded.") + } + + // if the test failed, and we have a panic information let's re-panic that + if t.Failed() && panicExecution != nil { + // we are about to panic, let's ensure we flush all ci visibility data and close the session event + integrations.ExitCiVisibility() + panic(fmt.Sprintf("test failed and panicked after %d retries.\n%v\n%v", executionIndex, panicExecution.panicData, panicExecution.panicStacktrace)) + } + } + } + } + + // Register the instrumented func as an internal instrumented func (to avoid double instrumentation) + setInstrumentationMetadata(runtime.FuncForPC(reflect.Indirect(reflect.ValueOf(wrapperFunc)).Pointer()), &instrumentationMetadata{IsInternal: true}) + return wrapperFunc +} + +//go:linkname testingTRunCleanup testing.(*common).runCleanup +func testingTRunCleanup(c *testing.T, ph int) (panicVal any) diff --git a/internal/civisibility/integrations/gotesting/reflections.go b/internal/civisibility/integrations/gotesting/reflections.go index 6aaac3ed4c..c01fc4f5f5 100644 --- a/internal/civisibility/integrations/gotesting/reflections.go +++ b/internal/civisibility/integrations/gotesting/reflections.go @@ -7,17 +7,25 @@ package gotesting import ( "errors" + "io" "reflect" "sync" + "sync/atomic" "testing" + "time" "unsafe" ) // getFieldPointerFrom gets an unsafe.Pointer (gc-safe type of pointer) to a struct field // useful to get or set values to private field func getFieldPointerFrom(value any, fieldName string) (unsafe.Pointer, error) { - indirectValue := reflect.Indirect(reflect.ValueOf(value)) - member := indirectValue.FieldByName(fieldName) + return getFieldPointerFromValue(reflect.Indirect(reflect.ValueOf(value)), fieldName) +} + +// getFieldPointerFromValue gets an unsafe.Pointer (gc-safe type of pointer) to a struct field +// useful to get or set values to private field +func getFieldPointerFromValue(value reflect.Value, fieldName string) (unsafe.Pointer, error) { + member := value.FieldByName(fieldName) if member.IsValid() { return unsafe.Pointer(member.UnsafeAddr()), nil } @@ -25,7 +33,61 @@ func getFieldPointerFrom(value any, fieldName string) (unsafe.Pointer, error) { return unsafe.Pointer(nil), errors.New("member is invalid") } +// copyFieldUsingPointers copies a private field value from one struct to another of the same type +func copyFieldUsingPointers[V any](source any, target any, fieldName string) error { + sourcePtr, err := getFieldPointerFrom(source, fieldName) + if err != nil { + return err + } + targetPtr, err := getFieldPointerFrom(target, fieldName) + if err != nil { + return err + } + + *(*V)(targetPtr) = *(*V)(sourcePtr) + return nil +} + +// **************** +// COMMON +// **************** + +// commonPrivateFields is collection of required private fields from testing.common +type commonPrivateFields struct { + mu *sync.RWMutex + level *int + name *string // Name of test or benchmark. + failed *bool // Test or benchmark has failed. + skipped *bool // Test or benchmark has been skipped. + parent *unsafe.Pointer // Parent common +} + +// AddLevel increase or decrease the testing.common.level field value, used by +// testing.B to create the name of the benchmark test +func (c *commonPrivateFields) AddLevel(delta int) int { + c.mu.Lock() + defer c.mu.Unlock() + *c.level = *c.level + delta + return *c.level +} + +// SetFailed set the boolean value in testing.common.failed field value +func (c *commonPrivateFields) SetFailed(value bool) { + c.mu.Lock() + defer c.mu.Unlock() + *c.failed = value +} + +// SetSkipped set the boolean value in testing.common.skipped field value +func (c *commonPrivateFields) SetSkipped(value bool) { + c.mu.Lock() + defer c.mu.Unlock() + *c.skipped = value +} + +// **************** // TESTING +// **************** // getInternalTestArray gets the pointer to the testing.InternalTest array inside a // testing.M instance containing all the "root" tests @@ -36,7 +98,108 @@ func getInternalTestArray(m *testing.M) *[]testing.InternalTest { return nil } +// getTestPrivateFields is a method to retrieve all required privates field from +// testing.T, returning a commonPrivateFields instance +func getTestPrivateFields(t *testing.T) *commonPrivateFields { + testFields := &commonPrivateFields{} + + // testing.common + if ptr, err := getFieldPointerFrom(t, "mu"); err == nil { + testFields.mu = (*sync.RWMutex)(ptr) + } + if ptr, err := getFieldPointerFrom(t, "level"); err == nil { + testFields.level = (*int)(ptr) + } + if ptr, err := getFieldPointerFrom(t, "name"); err == nil { + testFields.name = (*string)(ptr) + } + if ptr, err := getFieldPointerFrom(t, "failed"); err == nil { + testFields.failed = (*bool)(ptr) + } + if ptr, err := getFieldPointerFrom(t, "skipped"); err == nil { + testFields.skipped = (*bool)(ptr) + } + if ptr, err := getFieldPointerFrom(t, "parent"); err == nil { + testFields.parent = (*unsafe.Pointer)(ptr) + } + + return testFields +} + +// getTestParentPrivateFields is a method to retrieve all required parent privates field from +// testing.T.parent, returning a commonPrivateFields instance +func getTestParentPrivateFields(t *testing.T) *commonPrivateFields { + indirectValue := reflect.Indirect(reflect.ValueOf(t)) + member := indirectValue.FieldByName("parent") + if member.IsValid() { + value := member.Elem() + testFields := &commonPrivateFields{} + + // testing.common + if ptr, err := getFieldPointerFromValue(value, "mu"); err == nil { + testFields.mu = (*sync.RWMutex)(ptr) + } + if ptr, err := getFieldPointerFromValue(value, "level"); err == nil { + testFields.level = (*int)(ptr) + } + if ptr, err := getFieldPointerFromValue(value, "name"); err == nil { + testFields.name = (*string)(ptr) + } + if ptr, err := getFieldPointerFromValue(value, "failed"); err == nil { + testFields.failed = (*bool)(ptr) + } + if ptr, err := getFieldPointerFromValue(value, "skipped"); err == nil { + testFields.skipped = (*bool)(ptr) + } + + return testFields + } + return nil +} + +// copyTestWithoutParent tries to copy all private fields except the t.parent from a *testing.T to another +func copyTestWithoutParent(source *testing.T, target *testing.T) { + // Copy important field values + _ = copyFieldUsingPointers[[]byte](source, target, "output") // Output generated by test or benchmark. + _ = copyFieldUsingPointers[io.Writer](source, target, "w") // For flushToParent. + _ = copyFieldUsingPointers[bool](source, target, "ran") // Test or benchmark (or one of its subtests) was executed. + _ = copyFieldUsingPointers[bool](source, target, "failed") // Test or benchmark has failed. + _ = copyFieldUsingPointers[bool](source, target, "skipped") // Test or benchmark has been skipped. + _ = copyFieldUsingPointers[bool](source, target, "done") // Test is finished and all subtests have completed. + _ = copyFieldUsingPointers[map[uintptr]struct{}](source, target, "helperPCs") // functions to be skipped when writing file/line info + _ = copyFieldUsingPointers[map[string]struct{}](source, target, "helperNames") // helperPCs converted to function names + _ = copyFieldUsingPointers[[]func()](source, target, "cleanups") // optional functions to be called at the end of the test + _ = copyFieldUsingPointers[string](source, target, "cleanupName") // Name of the cleanup function. + _ = copyFieldUsingPointers[[]uintptr](source, target, "cleanupPc") // The stack trace at the point where Cleanup was called. + _ = copyFieldUsingPointers[bool](source, target, "finished") // Test function has completed. + _ = copyFieldUsingPointers[bool](source, target, "inFuzzFn") // Whether the fuzz target, if this is one, is running. + + _ = copyFieldUsingPointers[unsafe.Pointer](source, target, "chatty") // A copy of chattyPrinter, if the chatty flag is set. + _ = copyFieldUsingPointers[bool](source, target, "bench") // Whether the current test is a benchmark. + _ = copyFieldUsingPointers[atomic.Bool](source, target, "hasSub") // whether there are sub-benchmarks. + _ = copyFieldUsingPointers[atomic.Bool](source, target, "cleanupStarted") // Registered cleanup callbacks have started to execute + _ = copyFieldUsingPointers[string](source, target, "runner") // Function name of tRunner running the test. + _ = copyFieldUsingPointers[bool](source, target, "isParallel") // Whether the test is parallel. + + _ = copyFieldUsingPointers[int](source, target, "level") // Nesting depth of test or benchmark. + _ = copyFieldUsingPointers[[]uintptr](source, target, "creator") // If level > 0, the stack trace at the point where the parent called t.Run. + _ = copyFieldUsingPointers[string](source, target, "name") // Name of test or benchmark. + _ = copyFieldUsingPointers[unsafe.Pointer](source, target, "start") // Time test or benchmark started + _ = copyFieldUsingPointers[time.Duration](source, target, "duration") + _ = copyFieldUsingPointers[[]*testing.T](source, target, "sub") // Queue of subtests to be run in parallel. + _ = copyFieldUsingPointers[atomic.Int64](source, target, "lastRaceErrors") // Max value of race.Errors seen during the test or its subtests. + _ = copyFieldUsingPointers[atomic.Bool](source, target, "raceErrorLogged") + _ = copyFieldUsingPointers[string](source, target, "tempDir") + _ = copyFieldUsingPointers[error](source, target, "tempDirErr") + _ = copyFieldUsingPointers[int32](source, target, "tempDirSeq") + + _ = copyFieldUsingPointers[bool](source, target, "isEnvSet") + _ = copyFieldUsingPointers[unsafe.Pointer](source, target, "context") // For running tests and subtests. +} + +// **************** // BENCHMARKS +// **************** // get the pointer to the internal benchmark array // getInternalBenchmarkArray gets the pointer to the testing.InternalBenchmark array inside @@ -48,22 +211,6 @@ func getInternalBenchmarkArray(m *testing.M) *[]testing.InternalBenchmark { return nil } -// commonPrivateFields is collection of required private fields from testing.common -type commonPrivateFields struct { - mu *sync.RWMutex - level *int - name *string // Name of test or benchmark. -} - -// AddLevel increase or decrease the testing.common.level field value, used by -// testing.B to create the name of the benchmark test -func (c *commonPrivateFields) AddLevel(delta int) int { - c.mu.Lock() - defer c.mu.Unlock() - *c.level = *c.level + delta - return *c.level -} - // benchmarkPrivateFields is a collection of required private fields from testing.B // also contains a pointer to the original testing.B for easy access type benchmarkPrivateFields struct { @@ -80,7 +227,7 @@ func getBenchmarkPrivateFields(b *testing.B) *benchmarkPrivateFields { B: b, } - // common + // testing.common if ptr, err := getFieldPointerFrom(b, "mu"); err == nil { benchFields.mu = (*sync.RWMutex)(ptr) } @@ -90,8 +237,17 @@ func getBenchmarkPrivateFields(b *testing.B) *benchmarkPrivateFields { if ptr, err := getFieldPointerFrom(b, "name"); err == nil { benchFields.name = (*string)(ptr) } + if ptr, err := getFieldPointerFrom(b, "failed"); err == nil { + benchFields.failed = (*bool)(ptr) + } + if ptr, err := getFieldPointerFrom(b, "skipped"); err == nil { + benchFields.skipped = (*bool)(ptr) + } + if ptr, err := getFieldPointerFrom(b, "parent"); err == nil { + benchFields.parent = (*unsafe.Pointer)(ptr) + } - // benchmark + // testing.B if ptr, err := getFieldPointerFrom(b, "benchFunc"); err == nil { benchFields.benchFunc = (*func(b *testing.B))(ptr) } diff --git a/internal/civisibility/integrations/gotesting/testing.go b/internal/civisibility/integrations/gotesting/testing.go index 91cc246fc9..ac6092645c 100644 --- a/internal/civisibility/integrations/gotesting/testing.go +++ b/internal/civisibility/integrations/gotesting/testing.go @@ -14,6 +14,7 @@ import ( "time" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" + "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/constants" "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/integrations" "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/utils" ) @@ -129,22 +130,45 @@ func (ddm *M) instrumentInternalTests(internalTests *[]testing.InternalTest) { func (ddm *M) executeInternalTest(testInfo *testingTInfo) func(*testing.T) { originalFunc := runtime.FuncForPC(reflect.Indirect(reflect.ValueOf(testInfo.originalFunc)).Pointer()) instrumentedFunc := func(t *testing.T) { + // Get the metadata regarding the execution (in case is already created from the additional features) + execMeta := getTestMetadata(t) + if execMeta == nil { + // in case there's no additional features then we create the metadata for this execution and defer the disposal + execMeta = createTestMetadata(t) + defer deleteTestMetadata(t) + } + // Create or retrieve the module, suite, and test for CI visibility. module := session.GetOrCreateModuleWithFramework(testInfo.moduleName, testFramework, runtime.Version()) suite := module.GetOrCreateSuite(testInfo.suiteName) test := suite.CreateTest(testInfo.testName) test.SetTestFunc(originalFunc) - setCiVisibilityTest(t, test) + + // Set the CI Visibility test to the execution metadata + execMeta.test = test + + // If the execution is a retry we tag the test event + if execMeta.isARetry { + // Set the retry tag + test.SetTag(constants.TestIsRetry, "true") + } + defer func() { if r := recover(); r != nil { // Handle panic and set error information. - test.SetErrorInfo("panic", fmt.Sprint(r), utils.GetStacktrace(1)) + execMeta.panicData = r + execMeta.panicStacktrace = utils.GetStacktrace(1) + test.SetErrorInfo("panic", fmt.Sprint(r), execMeta.panicStacktrace) suite.SetTag(ext.Error, true) module.SetTag(ext.Error, true) test.Close(integrations.ResultStatusFail) - checkModuleAndSuite(module, suite) - integrations.ExitCiVisibility() - panic(r) + if !execMeta.hasAdditionalFeatureWrapper { + // we are going to let the additional feature wrapper to handle + // the panic, and module and suite closing (we don't want to close the suite earlier in case of a retry) + checkModuleAndSuite(module, suite) + integrations.ExitCiVisibility() + panic(r) + } } else { // Normal finalization: determine the test result based on its state. if t.Failed() { @@ -158,7 +182,11 @@ func (ddm *M) executeInternalTest(testInfo *testingTInfo) func(*testing.T) { test.Close(integrations.ResultStatusPass) } - checkModuleAndSuite(module, suite) + if !execMeta.hasAdditionalFeatureWrapper { + // we are going to let the additional feature wrapper to handle + // the module and suite closing (we don't want to close the suite earlier in case of a retry) + checkModuleAndSuite(module, suite) + } } }() @@ -166,8 +194,11 @@ func (ddm *M) executeInternalTest(testInfo *testingTInfo) func(*testing.T) { testInfo.originalFunc(t) } + // Register the instrumented func as an internal instrumented func (to avoid double instrumentation) setInstrumentationMetadata(runtime.FuncForPC(reflect.Indirect(reflect.ValueOf(instrumentedFunc)).Pointer()), &instrumentationMetadata{IsInternal: true}) - return instrumentedFunc + + // Get the additional feature wrapper + return applyAdditionalFeaturesToTestFunc(instrumentedFunc) } // instrumentInternalBenchmarks instruments the internal benchmarks for CI visibility. @@ -255,8 +286,17 @@ func (ddm *M) executeInternalBenchmark(benchmarkInfo *testingBInfo) func(*testin iPfOfB = getBenchmarkPrivateFields(b) // Replace the benchmark function with the original one (this must be executed only once - the first iteration[b.run1]). *iPfOfB.benchFunc = benchmarkInfo.originalFunc - // Set the CI visibility benchmark. - setCiVisibilityTest(b, test) + + // Get the metadata regarding the execution (in case is already created from the additional features) + execMeta := getTestMetadata(b) + if execMeta == nil { + // in case there's no additional features then we create the metadata for this execution and defer the disposal + execMeta = createTestMetadata(b) + defer deleteTestMetadata(b) + } + + // Sets the CI Visibility test + execMeta.test = test // Restart the timer and execute the original benchmark function. b.ResetTimer() diff --git a/internal/civisibility/integrations/gotesting/testingB.go b/internal/civisibility/integrations/gotesting/testingB.go index b37bef009d..5678daf6f6 100644 --- a/internal/civisibility/integrations/gotesting/testingB.go +++ b/internal/civisibility/integrations/gotesting/testingB.go @@ -54,7 +54,7 @@ func (ddb *B) Run(name string, f func(*testing.B)) bool { // integration tests. func (ddb *B) Context() context.Context { b := (*testing.B)(ddb) - ciTestItem := getCiVisibilityTest(b) + ciTestItem := getTestMetadata(b) if ciTestItem != nil && ciTestItem.test != nil { return ciTestItem.test.Context() } diff --git a/internal/civisibility/integrations/gotesting/testingT.go b/internal/civisibility/integrations/gotesting/testingT.go index e05f60abc8..74c2d5cee0 100644 --- a/internal/civisibility/integrations/gotesting/testingT.go +++ b/internal/civisibility/integrations/gotesting/testingT.go @@ -40,7 +40,7 @@ func (ddt *T) Run(name string, f func(*testing.T)) bool { // integration tests. func (ddt *T) Context() context.Context { t := (*testing.T)(ddt) - ciTestItem := getCiVisibilityTest(t) + ciTestItem := getTestMetadata(t) if ciTestItem != nil && ciTestItem.test != nil { return ciTestItem.test.Context() } diff --git a/internal/civisibility/integrations/gotesting/testing_test.go b/internal/civisibility/integrations/gotesting/testing_test.go index b780815bdf..0dcbdb559d 100644 --- a/internal/civisibility/integrations/gotesting/testing_test.go +++ b/internal/civisibility/integrations/gotesting/testing_test.go @@ -6,6 +6,7 @@ package gotesting import ( + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -21,6 +22,7 @@ import ( ddtracer "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/constants" "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/integrations" + "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/utils/net" "github.com/stretchr/testify/assert" ) @@ -30,51 +32,98 @@ var mTracer mocktracer.Tracer // TestMain is the entry point for testing and runs before any test. func TestMain(m *testing.M) { + + // mock the settings api to enable automatic test retries + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Printf("MockApi received request: %s\n", r.URL.Path) + + // Settings request + if r.URL.Path == "/api/v2/libraries/tests/services/setting" { + w.Header().Set("Content-Type", "application/json") + response := struct { + Data struct { + ID string `json:"id"` + Type string `json:"type"` + Attributes net.SettingsResponseData `json:"attributes"` + } `json:"data,omitempty"` + }{} + + // let's enable flaky test retries + response.Data.Attributes = net.SettingsResponseData{ + FlakyTestRetriesEnabled: true, + } + + fmt.Printf("MockApi sending response: %v\n", response) + json.NewEncoder(w).Encode(&response) + } + })) + defer server.Close() + + // set the custom agentless url and the flaky retry count env-var + fmt.Printf("Using mockapi at: %s\n", server.URL) + os.Setenv(constants.CIVisibilityAgentlessEnabledEnvironmentVariable, "1") + os.Setenv(constants.CIVisibilityAgentlessURLEnvironmentVariable, server.URL) + os.Setenv(constants.APIKeyEnvironmentVariable, "12345") + os.Setenv(constants.CIVisibilityFlakyRetryCountEnvironmentVariable, "10") + + // initialize the mock tracer for doing assertions on the finished spans currentM = m mTracer = integrations.InitializeCIVisibilityMock() - // (*M)(m).Run() cast m to gotesting.M and just run - // or use a helper method gotesting.RunM(m) - - // os.Exit((*M)(m).Run()) - exit := RunM(m) - if exit != 0 { - os.Exit(exit) - } + // execute the tests, because we are expecting some tests to fail and check the assertion later + // we don't store the exit code from the test runner + RunM(m) + // get all finished spans finishedSpans := mTracer.FinishedSpans() + // 1 session span // 1 module span - // 1 suite span (optional 1 from reflections_test.go) - // 6 tests spans - // 7 sub stest spans - // 2 normal spans (from integration tests) - // 1 benchmark span (optional - require the -bench option) - if len(finishedSpans) < 17 { - panic("expected at least 17 finished spans, got " + strconv.Itoa(len(finishedSpans))) + // 2 suite span (testing_test.go and reflections_test.go) + // 5 tests spans from testing_test.go + // 7 sub stest spans from testing_test.go + // 1 TestRetryWithPanic + 3 retry tests from testing_test.go + // 1 TestRetryWithFail + 3 retry tests from testing_test.go + // 1 TestRetryAlwaysFail + 10 retry tests from testing_test.go + // 2 normal spans from testing_test.go + // 5 tests from reflections_test.go + // 2 benchmark spans (optional - require the -bench option) + fmt.Printf("Number of spans received: %d\n", len(finishedSpans)) + if len(finishedSpans) < 36 { + panic("expected at least 36 finished spans, got " + strconv.Itoa(len(finishedSpans))) } sessionSpans := getSpansWithType(finishedSpans, constants.SpanTypeTestSession) + fmt.Printf("Number of sessions received: %d\n", len(sessionSpans)) + showResourcesNameFromSpans(sessionSpans) if len(sessionSpans) != 1 { panic("expected exactly 1 session span, got " + strconv.Itoa(len(sessionSpans))) } moduleSpans := getSpansWithType(finishedSpans, constants.SpanTypeTestModule) + fmt.Printf("Number of modules received: %d\n", len(moduleSpans)) + showResourcesNameFromSpans(moduleSpans) if len(moduleSpans) != 1 { panic("expected exactly 1 module span, got " + strconv.Itoa(len(moduleSpans))) } suiteSpans := getSpansWithType(finishedSpans, constants.SpanTypeTestSuite) - if len(suiteSpans) < 1 { - panic("expected at least 1 suite span, got " + strconv.Itoa(len(suiteSpans))) + fmt.Printf("Number of suites received: %d\n", len(suiteSpans)) + showResourcesNameFromSpans(suiteSpans) + if len(suiteSpans) != 2 { + panic("expected exactly 2 suite spans, got " + strconv.Itoa(len(suiteSpans))) } testSpans := getSpansWithType(finishedSpans, constants.SpanTypeTest) - if len(testSpans) < 12 { - panic("expected at least 12 suite span, got " + strconv.Itoa(len(testSpans))) + fmt.Printf("Number of tests received: %d\n", len(testSpans)) + showResourcesNameFromSpans(testSpans) + if len(testSpans) != 36 { + panic("expected exactly 36 test spans, got " + strconv.Itoa(len(testSpans))) } httpSpans := getSpansWithType(finishedSpans, ext.SpanTypeHTTP) + fmt.Printf("Number of http spans received: %d\n", len(httpSpans)) + showResourcesNameFromSpans(httpSpans) if len(httpSpans) != 2 { panic("expected exactly 2 normal spans, got " + strconv.Itoa(len(httpSpans))) } @@ -223,6 +272,45 @@ func TestSkip(gt *testing.T) { t.Skip("Nothing to do here, skipping!") } +// Tests for test retries feature + +var testRetryWithPanicRunNumber = 0 + +func TestRetryWithPanic(t *testing.T) { + t.Cleanup(func() { + if testRetryWithPanicRunNumber == 1 { + fmt.Println("CleanUp from the initial execution") + } else { + fmt.Println("CleanUp from the retry") + } + }) + testRetryWithPanicRunNumber++ + if testRetryWithPanicRunNumber < 4 { + panic("Test Panic") + } +} + +var testRetryWithFailRunNumber = 0 + +func TestRetryWithFail(t *testing.T) { + t.Cleanup(func() { + if testRetryWithFailRunNumber == 1 { + fmt.Println("CleanUp from the initial execution") + } else { + fmt.Println("CleanUp from the retry") + } + }) + testRetryWithFailRunNumber++ + if testRetryWithFailRunNumber < 4 { + t.Fatal("Failed due the wrong execution number") + } +} + +func TestRetryAlwaysFail(t *testing.T) { + t.Parallel() + t.Fatal("Always fail") +} + // BenchmarkFirst demonstrates benchmark instrumentation with sub-benchmarks. func BenchmarkFirst(gb *testing.B) { @@ -366,3 +454,9 @@ func getSpansWithType(spans []mocktracer.Span, spanType string) []mocktracer.Spa return result } + +func showResourcesNameFromSpans(spans []mocktracer.Span) { + for i, span := range spans { + fmt.Printf(" [%d] = %v\n", i, span.Tag(ext.ResourceName)) + } +} diff --git a/internal/civisibility/utils/names.go b/internal/civisibility/utils/names.go index 1edcb2b972..94eacdb189 100644 --- a/internal/civisibility/utils/names.go +++ b/internal/civisibility/utils/names.go @@ -10,9 +10,15 @@ import ( "fmt" "path/filepath" "runtime" + "slices" "strings" ) +var ( + // ignoredFunctionsFromStackTrace array with functions we want to ignore on the final stacktrace (because doesn't add anything useful) + ignoredFunctionsFromStackTrace = []string{"runtime.gopanic", "runtime.panicmem", "runtime.sigpanic"} +) + // GetModuleAndSuiteName extracts the module name and suite name from a given program counter (pc). // This function utilizes runtime.FuncForPC to retrieve the full function name associated with the // program counter, then splits the string to separate the package name from the function name. @@ -72,6 +78,11 @@ func GetStacktrace(skip int) string { buffer := new(bytes.Buffer) for { if frame, ok := frames.Next(); ok { + // let's check if we need to ignore this frame + if slices.Contains(ignoredFunctionsFromStackTrace, frame.Function) { + continue + } + // writing frame to the buffer _, _ = fmt.Fprintf(buffer, "%s\n\t%s:%d\n", frame.Function, frame.File, frame.Line) } else { break diff --git a/internal/civisibility/utils/net/client.go b/internal/civisibility/utils/net/client.go index 60e1c28683..366aa9409c 100644 --- a/internal/civisibility/utils/net/client.go +++ b/internal/civisibility/utils/net/client.go @@ -65,7 +65,7 @@ type ( var _ Client = &client{} -func NewClient() Client { +func NewClientWithServiceName(serviceName string) Client { ciTags := utils.GetCITags() // get the environment @@ -75,16 +75,18 @@ func NewClient() Client { } // get the service name - serviceName := os.Getenv("DD_SERVICE") if serviceName == "" { - if repoURL, ok := ciTags[constants.GitRepositoryURL]; ok { - // regex to sanitize the repository url to be used as a service name - repoRegex := regexp.MustCompile(`(?m)/([a-zA-Z0-9\-_.]*)$`) - matches := repoRegex.FindStringSubmatch(repoURL) - if len(matches) > 1 { - repoURL = strings.TrimSuffix(matches[1], ".git") + serviceName = os.Getenv("DD_SERVICE") + if serviceName == "" { + if repoURL, ok := ciTags[constants.GitRepositoryURL]; ok { + // regex to sanitize the repository url to be used as a service name + repoRegex := regexp.MustCompile(`(?m)/([a-zA-Z0-9\-_.]*)$`) + matches := repoRegex.FindStringSubmatch(repoURL) + if len(matches) > 1 { + repoURL = strings.TrimSuffix(matches[1], ".git") + } + serviceName = repoURL } - serviceName = repoURL } } @@ -204,6 +206,10 @@ func NewClient() Client { } } +func NewClient() Client { + return NewClientWithServiceName("") +} + func (c *client) getURLPath(urlPath string) string { if c.agentless { return fmt.Sprintf("%s/%s", c.baseURL, urlPath)