Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 17 additions & 4 deletions functional_tests/functional/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,7 @@ func testNodeJSTraces(t *testing.T) {
expectedTracesFile := filepath.Join(testDir, expectedValuesDir, "expected_nodejs_traces.yaml")
expectedTraces, err := golden.ReadTraces(expectedTracesFile)
require.NoError(t, err)
internal.ClearTraceSchemaURLs(expectedTraces)

internal.WaitForTraces(t, 10, tracesConsumer)

Expand All @@ -704,6 +705,7 @@ func testNodeJSTraces(t *testing.T) {

maskScopeVersion(*selectedTrace)
maskScopeVersion(expectedTraces)
internal.ClearTraceSchemaURLs(*selectedTrace)

internal.MaybeWriteUpdateExpectedTracesResults(t, expectedTracesFile, selectedTrace)
err = ptracetest.CompareTraces(expectedTraces, *selectedTrace,
Expand Down Expand Up @@ -749,6 +751,7 @@ func testPythonTraces(t *testing.T) {
expectedTracesFile := filepath.Join(testDir, expectedValuesDir, "expected_python_traces.yaml")
expectedTraces, err := golden.ReadTraces(expectedTracesFile)
require.NoError(t, err)
internal.ClearTraceSchemaURLs(expectedTraces)

internal.WaitForTraces(t, 10, tracesConsumer)

Expand All @@ -772,6 +775,7 @@ func testPythonTraces(t *testing.T) {

maskScopeVersion(*selectedTrace)
maskScopeVersion(expectedTraces)
internal.ClearTraceSchemaURLs(*selectedTrace)

internal.MaybeWriteUpdateExpectedTracesResults(t, expectedTracesFile, selectedTrace)
err = ptracetest.CompareTraces(expectedTraces, *selectedTrace,
Expand Down Expand Up @@ -817,6 +821,7 @@ func testJavaTraces(t *testing.T) {
expectedTracesFile := filepath.Join(testDir, expectedValuesDir, "expected_java_traces.yaml")
expectedTraces, err := golden.ReadTraces(expectedTracesFile)
require.NoError(t, err)
internal.ClearTraceSchemaURLs(expectedTraces)

internal.WaitForTraces(t, 10, tracesConsumer)

Expand All @@ -838,6 +843,7 @@ func testJavaTraces(t *testing.T) {

maskScopeVersion(*selectedTrace)
maskScopeVersion(expectedTraces)
internal.ClearTraceSchemaURLs(*selectedTrace)

internal.MaybeWriteUpdateExpectedTracesResults(t, expectedTracesFile, selectedTrace)
err = ptracetest.CompareTraces(expectedTraces, *selectedTrace,
Expand Down Expand Up @@ -881,6 +887,7 @@ func testDotNetTraces(t *testing.T) {
expectedTracesFile := filepath.Join(testDir, expectedValuesDir, "expected_dotnet_traces.yaml")
expectedTraces, err := golden.ReadTraces(expectedTracesFile)
require.NoError(t, err)
internal.ClearTraceSchemaURLs(expectedTraces)

internal.WaitForTraces(t, 30, tracesConsumer)
var selectedTrace *ptrace.Traces
Expand All @@ -903,6 +910,7 @@ func testDotNetTraces(t *testing.T) {
maskScopeVersion(expectedTraces)
maskSpanParentID(*selectedTrace)
maskSpanParentID(expectedTraces)
internal.ClearTraceSchemaURLs(*selectedTrace)

internal.MaybeWriteUpdateExpectedTracesResults(t, expectedTracesFile, selectedTrace)
err = ptracetest.CompareTraces(expectedTraces, *selectedTrace,
Expand Down Expand Up @@ -986,7 +994,7 @@ func testK8sClusterReceiverMetrics(t *testing.T) {
require.NoError(t, err, "Failed to read expected metrics from expected_cluster_receiver.yaml")

targetMetric := "k8s.pod.phase"
selectedMetrics := internal.SelectMetricSetWithTimeout(t, expectedMetrics, targetMetric, metricsConsumer, false, 3*time.Minute, 10*time.Second)
selectedMetrics, exactMatch := internal.SelectMetricSetWithTimeout(t, expectedMetrics, targetMetric, metricsConsumer, 3*time.Minute, 10*time.Second)
require.NotNil(t, selectedMetrics, "No metrics batch found containing target metric: %s", targetMetric)

metricNames := internal.GetMetricNames(&expectedMetrics)
Expand Down Expand Up @@ -1016,6 +1024,9 @@ func testK8sClusterReceiverMetrics(t *testing.T) {
pmetrictest.IgnoreSubsequentDataPoints("k8s.container.ready", "k8s.container.restarts", "k8s.pod.phase"),
)
if err != nil {
if !exactMatch {
t.Logf("No exact count match: expected %d metrics, selected payload has %d", expectedMetrics.MetricCount(), selectedMetrics.MetricCount())
}
internal.MaybeUpdateExpectedMetricsResults(t, expectedMetricsFile, selectedMetrics)
require.NoError(t, err, "K8s cluster receiver metrics comparison failed. Error: %v", err)
}
Expand Down Expand Up @@ -1244,7 +1255,7 @@ func testAgentMetricsTemplate(t *testing.T, metricsSink *consumertest.MetricsSin
expectedMetrics, err := golden.ReadMetrics(expectedMetricsFile)
require.NoError(t, err, "Failed to read expected metrics from %s", expectedFileName)

selectedMetrics := internal.SelectMetricSetWithTimeout(t, expectedMetrics, targetMetric, metricsSink, false, 3*time.Minute, 10*time.Second)
selectedMetrics, exactMatch := internal.SelectMetricSetWithTimeout(t, expectedMetrics, targetMetric, metricsSink, 3*time.Minute, 10*time.Second)
require.NotNil(t, selectedMetrics, "No metrics batch found containing target metric: %s", targetMetric)

testName := t.Name()
Expand All @@ -1254,13 +1265,15 @@ func testAgentMetricsTemplate(t *testing.T, metricsSink *consumertest.MetricsSin

err = tryMetricsComparison(expectedMetrics, *selectedMetrics)
if err != nil {
if !exactMatch {
t.Logf("No exact count match: expected %d metrics, selected payload has %d", expectedMetrics.MetricCount(), selectedMetrics.MetricCount())
}
t.Logf("Metric comparison failed for %s: %v", testName, err)
internal.MaybeUpdateExpectedMetricsResults(t, expectedMetricsFile, selectedMetrics)
require.NoError(t, err, "Metric comparison failed for %s test. Error: %v", testName, err)
}

metricCount := selectedMetrics.MetricCount()
t.Logf("Metric comparison passed for %d metrics in %s test", metricCount, testName)
t.Logf("Metric comparison passed for %d metrics in %s test", selectedMetrics.MetricCount(), testName)
}

// tryMetricsComparison performs metric comparison using pmetrictest.CompareMetrics and returns error
Expand Down
84 changes: 56 additions & 28 deletions functional_tests/internal/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ func MaybeWriteUpdateExpectedTracesResults(t *testing.T, file string, traces *pt
}
}

// ClearTraceSchemaURLs strips schema URLs from all ResourceSpans and ScopeSpans
func ClearTraceSchemaURLs(td ptrace.Traces) {
for i := 0; i < td.ResourceSpans().Len(); i++ {
td.ResourceSpans().At(i).SetSchemaUrl("")
for j := 0; j < td.ResourceSpans().At(i).ScopeSpans().Len(); j++ {
td.ResourceSpans().At(i).ScopeSpans().At(j).SetSchemaUrl("")
}
}
}

func MaybeUpdateExpectedMetricsResults(t *testing.T, file string, metrics *pmetric.Metrics) {
if shouldUpdateExpectedResults() {
require.NoError(t, golden.WriteMetrics(t, file, *metrics))
Expand Down Expand Up @@ -420,49 +430,67 @@ func DeleteObject(t *testing.T, k8sClient *k8stest.K8sClient, objYAML string) {
}
}

// SelectMetricSet finds a metrics payload containing a target metric without any re-checking logic.
func SelectMetricSet(t *testing.T, expected pmetric.Metrics, targetMetric string, metricSink *consumertest.MetricsSink, ignoreLen bool) *pmetric.Metrics {
var selectedMetrics *pmetric.Metrics
// SelectMetricSet finds a metrics payload containing a target metric.
// It first tries to find a payload with an exact ResourceMetrics and MetricCount
// match to expected (best candidate for CompareMetrics). If none is found, it
// falls back to the payload with the highest MetricCount (most complete batch,
// suitable for updating golden files).
// Returns the selected payload and whether it was an exact match.
func SelectMetricSet(t *testing.T, expected pmetric.Metrics, targetMetric string, metricSink *consumertest.MetricsSink) (*pmetric.Metrics, bool) {
var exactMatch *pmetric.Metrics
var fallback *pmetric.Metrics
Comment on lines +440 to +441
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps I'm being guided by the diff above in which exactMatch was a boolean, but felt that this nit makes more readable:

Suggested change
var exactMatch *pmetric.Metrics
var fallback *pmetric.Metrics
var exactMatchMetrics *pmetric.Metrics
var fallbackMetrics *pmetric.Metrics

fallbackCount := -1

for h := len(metricSink.AllMetrics()) - 1; h >= 0; h-- {
m := metricSink.AllMetrics()[h]
foundTargetMetric := false

OUTER:
for i := 0; i < m.ResourceMetrics().Len(); i++ {
for j := 0; j < m.ResourceMetrics().At(i).ScopeMetrics().Len(); j++ {
for k := 0; k < m.ResourceMetrics().At(i).ScopeMetrics().At(j).Metrics().Len(); k++ {
metric := m.ResourceMetrics().At(i).ScopeMetrics().At(j).Metrics().At(k)
if metric.Name() == targetMetric {
foundTargetMetric = true
break OUTER
}
}
}
}

if !foundTargetMetric {
if !containsMetric(m, targetMetric) {
continue
}

if ignoreLen || (m.ResourceMetrics().Len() == expected.ResourceMetrics().Len() && m.MetricCount() == expected.MetricCount()) {
selectedMetrics = &m
t.Logf("Found target metric '%s' in payload with %d total metrics", targetMetric, m.MetricCount())
if m.ResourceMetrics().Len() == expected.ResourceMetrics().Len() && m.MetricCount() == expected.MetricCount() {
exactMatch = &m
break
}
if m.MetricCount() > fallbackCount {
fallback = &m
fallbackCount = m.MetricCount()
Comment on lines 444 to +455
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

SelectMetricSet stores &m where m is the loop variable (m := metricSink.AllMetrics()[h]). In Go this results in returning a pointer to the same variable reused across iterations, so exactMatch/fallback can end up pointing at the wrong payload. Create a per-iteration copy before taking the address (or store the index and take the address of a stable slice element).

Copilot uses AI. Check for mistakes.
}
}

if exactMatch != nil {
t.Logf("Selected exact-match payload with target metric '%s': %d metrics, %d resources",
targetMetric, exactMatch.MetricCount(), exactMatch.ResourceMetrics().Len())
return exactMatch, true
}
if fallback != nil {
t.Logf("No exact match for expected counts (%d metrics, %d resources); selected best-effort payload with '%s': %d metrics, %d resources",
expected.MetricCount(), expected.ResourceMetrics().Len(), targetMetric, fallback.MetricCount(), fallback.ResourceMetrics().Len())
}
return fallback, false
}

return selectedMetrics
func containsMetric(m pmetric.Metrics, name string) bool {
for i := 0; i < m.ResourceMetrics().Len(); i++ {
for j := 0; j < m.ResourceMetrics().At(i).ScopeMetrics().Len(); j++ {
for k := 0; k < m.ResourceMetrics().At(i).ScopeMetrics().At(j).Metrics().Len(); k++ {
if m.ResourceMetrics().At(i).ScopeMetrics().At(j).Metrics().At(k).Name() == name {
return true
}
}
}
}
return false
}

// SelectMetricSetWithTimeout finds a metrics payload containing a target metric with Eventually timeout
func SelectMetricSetWithTimeout(t *testing.T, expected pmetric.Metrics, targetMetric string, metricSink *consumertest.MetricsSink, ignoreLen bool, timeout time.Duration, interval time.Duration) *pmetric.Metrics {
// SelectMetricSetWithTimeout retries SelectMetricSet until a payload is found or the timeout expires.
// Returns the selected payload and whether it was an exact match on counts.
func SelectMetricSetWithTimeout(t *testing.T, expected pmetric.Metrics, targetMetric string, metricSink *consumertest.MetricsSink, timeout time.Duration, interval time.Duration) (*pmetric.Metrics, bool) {
var selectedMetrics *pmetric.Metrics
var exactMatch bool

require.Eventuallyf(t, func() bool {
selectedMetrics = SelectMetricSet(t, expected, targetMetric, metricSink, ignoreLen)
selectedMetrics, exactMatch = SelectMetricSet(t, expected, targetMetric, metricSink)
return selectedMetrics != nil
}, timeout, interval, "Failed to find target metric %s within timeout period of %v", targetMetric, timeout)
Comment on lines 490 to 493
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

SelectMetricSetWithTimeout stops polling as soon as any payload containing the target metric exists (return selectedMetrics != nil). Since SelectMetricSet now returns a non-nil fallback even when counts don’t match, this can return early with an incomplete batch and miss a later exact-match / more complete batch that arrives before the timeout. Consider continuing to retry until an exact match is found (tracking the best fallback seen so far) and only returning the fallback after the timeout expires.

Copilot uses AI. Check for mistakes.

return selectedMetrics
return selectedMetrics, exactMatch
}
10 changes: 6 additions & 4 deletions functional_tests/istio/istio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,17 +318,17 @@ func Test_IstioMetrics(t *testing.T) {
flakyMetrics := []string{"galley_validation_config_update_error"} // only shows up when config validation fails - removed if present when comparing
t.Run("istiod metrics captured", func(t *testing.T) {
testIstioMetrics(t, "testdata/expected_istiod.yaml", "pilot_services",
flakyMetrics, true, metricsSink)
flakyMetrics, metricsSink)
})

flakyMetrics = []string{"istio_agent_pilot_xds_expired_nonce"}
t.Run("istio ingress metrics captured", func(t *testing.T) {
testIstioMetrics(t, "testdata/expected_istioingress.yaml",
"istio_requests_total", flakyMetrics, true, metricsSink)
"istio_requests_total", flakyMetrics, metricsSink)
})
}

func testIstioMetrics(t *testing.T, expectedMetricsFile string, includeMetricName string, flakyMetricNames []string, ignoreLen bool, metricsSink *consumertest.MetricsSink) {
func testIstioMetrics(t *testing.T, expectedMetricsFile string, includeMetricName string, flakyMetricNames []string, metricsSink *consumertest.MetricsSink) {
expectedMetrics, err := golden.ReadMetrics(expectedMetricsFile)
require.NoError(t, err)

Expand Down Expand Up @@ -377,7 +377,7 @@ func testIstioMetrics(t *testing.T, expectedMetricsFile string, includeMetricNam
t.Logf("Comparison error: %v", err)
}

selectedMetrics := internal.SelectMetricSet(t, expectedMetrics, includeMetricName, metricsSink, ignoreLen)
selectedMetrics, _ := internal.SelectMetricSet(t, expectedMetrics, includeMetricName, metricsSink)
if selectedMetrics != nil {
internal.MaybeUpdateExpectedMetricsResults(t, expectedMetricsFile, selectedMetrics)
}
Expand Down Expand Up @@ -413,6 +413,7 @@ func testIstioHTTPBinTraces(t *testing.T, expectedTracesFile string, tracesSink
expectedTraces, err := golden.ReadTraces(expectedTracesFile)
require.NoError(t, err)
require.NotEmpty(t, expectedTraces)
internal.ClearTraceSchemaURLs(expectedTraces)

requests := []request{
{"http://httpbin.example.com/status/200", "httpbin.example.com", "httpbin.example.com:80", "/status/200"},
Expand Down Expand Up @@ -456,6 +457,7 @@ func testIstioHTTPBinTraces(t *testing.T, expectedTracesFile string, tracesSink
span.CopyTo(css.Spans().AppendEmpty())
lastCandidate = candidate

internal.ClearTraceSchemaURLs(candidate)
err = ptracetest.CompareTraces(expectedTraces, candidate,
ptracetest.IgnoreResourceSpansOrder(),
ptracetest.IgnoreSpansOrder(),
Expand Down
Loading