[test] Upgrade span_tags_trace.json from v1 model to ptrace.Traces#8044
[test] Upgrade span_tags_trace.json from v1 model to ptrace.Traces#8044yurishkuro merged 19 commits intojaegertracing:mainfrom
span_tags_trace.json from v1 model to ptrace.Traces#8044Conversation
Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8044 +/- ##
==========================================
+ Coverage 95.49% 95.50% +0.01%
==========================================
Files 316 316
Lines 16756 16756
==========================================
+ Hits 16001 16003 +2
+ Misses 590 589 -1
+ Partials 165 164 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Metrics Comparison SummaryTotal changes across all snapshots: 69 Detailed changes per snapshotsummary_metrics_snapshot_elasticsearch📊 Metrics Diff SummaryTotal Changes: 37
❌ Removed Metrics
View diff sample-jaeger_storage_latency_seconds{le="+Inf",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
-jaeger_storage_latency_seconds{le="0",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
-jaeger_storage_latency_seconds{le="10",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
-jaeger_storage_latency_seconds{le="100",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
-jaeger_storage_latency_seconds{le="1000",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
-jaeger_storage_latency_seconds{le="10000",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
-jaeger_storage_latency_seconds{le="25",name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}
...View diff sample-jaeger_storage_requests{name="some_storage",operation="find_traces",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",result="err",role="tracestore"}View diff sample-rpc_server_call_duration_seconds{le="+Inf",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
-rpc_server_call_duration_seconds{le="0",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
-rpc_server_call_duration_seconds{le="10",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
-rpc_server_call_duration_seconds{le="100",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
-rpc_server_call_duration_seconds{le="1000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
-rpc_server_call_duration_seconds{le="10000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
-rpc_server_call_duration_seconds{le="25",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.39.0",otel_scope_version="0.65.0",rpc_method="jaeger.api_v3.QueryService/FindTraces",rpc_response_status_code="Unknown",rpc_system_name="grpc"}
...Total Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_badger📊 Metrics Diff SummaryTotal Changes: 32
❌ Removed Metrics
View diff sample-jaeger_storage_badger_compaction_current_num_lsm{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_compaction_current_num_lsm{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_get_num_memtable{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_get_num_memtable{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_get_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_get_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_get_with_result_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_get_with_result_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_iterator_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_iterator_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_put_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_put_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_read_bytes_lsm{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_read_bytes_lsm{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_read_bytes_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_read_bytes_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_read_num_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_read_num_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_size_bytes_lsm{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_size_bytes_lsm{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_size_bytes_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_size_bytes_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_bytes_l0{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_bytes_l0{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_bytes_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_bytes_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_bytes_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_bytes_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_num_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_num_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_pending_num_memtable{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_pending_num_memtable{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"} |
There was a problem hiding this comment.
Pull request overview
Updates the storage integration test harness to support trace fixtures stored directly as OTLP JSON (ptrace.Traces), enabling incremental migration of existing v1-model fixtures toward the v2 storage API test flow (part of #7050).
Changes:
- Adds an
IsOtelTraceflag to query fixtures and threads it through fixture loading to choose the correct unmarshal path (v1 model JSONPB vs OTLP JSON). - Migrates
span_tags_trace.jsonfrom v1 model JSON to OTLP JSON and updatesqueries.jsonto mark it as OTLP. - Introduces timestamp normalization for OTLP traces (
dateOffsetNormalizer) plus a unit test.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/storage/integration/integration.go | Adds OTLP fixture loading path and plumbs an IsOtelTrace switch through the integration tests. |
| internal/storage/integration/fixtures/traces/span_tags_trace.json | Converts one trace fixture to OTLP JSON (resourceSpans/scopeSpans/spans). |
| internal/storage/integration/fixtures/queries.json | Marks the migrated fixture as OTLP via IsOtelTrace. |
| internal/storage/integration/dates.go | Adds date normalization logic for OTLP traces to keep fixtures “recent” for queries. |
| internal/storage/integration/dates_test.go | Adds unit test coverage for the date normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func Test_dateOffsetNormalizer(t *testing.T) { | ||
| origTime := time.Date( | ||
| 2024, time.January, 10, | ||
| 14, 30, 45, 123456789, | ||
| time.UTC, | ||
| ) | ||
| origStartTime := time.Date( | ||
| 2017, time.January, 25, | ||
| 23, 56, 31, 639875000, | ||
| time.UTC, | ||
| ) | ||
| origTs := pcommon.NewTimestampFromTime(origTime) | ||
| origStartTs := pcommon.NewTimestampFromTime(origStartTime) | ||
| twoDaysAgo := -2 | ||
| oneDayAgo := -1 | ||
| now := time.Now() | ||
| expectedTwoDaysAgo := now.UTC().AddDate(0, 0, twoDaysAgo) | ||
| expectedOneDayAgo := now.UTC().AddDate(0, 0, oneDayAgo) | ||
| normalizer := newDateOffsetNormalizer(now) | ||
| td := ptrace.NewTraces() | ||
| span := td.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty().Spans().AppendEmpty() | ||
| span.SetStartTimestamp(origStartTs) | ||
| span.SetEndTimestamp(origTs) | ||
| event := span.Events().AppendEmpty() | ||
| event.SetTimestamp(origTs) | ||
| normalizer.normalizeTrace(td) |
There was a problem hiding this comment.
Test_dateOffsetNormalizer only covers the single 2017-01-25 timestamp that matches the hard-coded UnixNano check. If the normalizer is meant to apply a -2 day offset to any timestamp on 2017-01-25 (per the docstring), add a test case with a different time on 2017-01-25 to prevent regressions when more OTLP fixtures are migrated.
There was a problem hiding this comment.
my attempt was to create a v2 alternative to this:
jaeger/internal/storage/integration/integration.go
Lines 510 to 518 in ed0527d
| tm := t.AsTime() | ||
| offset := -1 | ||
| // Apply a -2 day offset for any timestamp whose UTC date is 2017-01-25. | ||
| // All other timestamps use the default -1 day offset to preserve existing behavior. | ||
| yearOrig, monthOrig, dayOrig := tm.UTC().Date() | ||
| if yearOrig == 2017 && monthOrig == time.January && dayOrig == 25 { | ||
| offset = -2 | ||
| } | ||
| year, month, day := d.tm.UTC().AddDate(0, 0, offset).Date() |
There was a problem hiding this comment.
I am not very confident of giving the responsibility of handling offsets to normalizer. The other approach could be giving the traces logic to tests and accepting offset as parameter for normalizeTime
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func correctTimeForTrace(td ptrace.Traces) { | ||
| now := time.Now().UTC() | ||
| normalizer := newDateOffsetNormalizer(now) | ||
| normalizer.normalizeTrace(td) | ||
| } |
There was a problem hiding this comment.
correctTimeForTrace calls time.Now() internally, while other fixtures/queries are also normalized via correctTime() (which also calls time.Now() internally). If these happen to straddle a UTC day boundary (e.g., CI running around midnight), query time ranges and trace timestamps can be normalized to different dates and cause rare test flakes. Consider computing a single reference now once per test run (or per StorageIntegration) and passing it into both JSON and OTLP normalization paths so all fixtures/queries use the same day offsets.
There was a problem hiding this comment.
catch is good but it will require more refactoring! should we do this @yurishkuro? because correctTime will be removed after all fixtures are upgraded
Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/storage/integration/fixtures/traces/span_tags_trace.json
Outdated
Show resolved
Hide resolved
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func correctTimeForTrace(td ptrace.Traces) { | ||
| now := time.Now().UTC() | ||
| normalizer := newDateOffsetNormalizer(now) | ||
| normalizer.normalizeTrace(td) | ||
| } |
There was a problem hiding this comment.
correctTime() and correctTimeForTrace() both call time.Now() internally. In testFindTraces the query fixtures are normalized via correctTime() while trace fixtures are normalized via correctTimeForTrace(), so if the test run crosses a UTC day boundary (or multiple fixtures are loaded far apart in time) the query time range and trace timestamps can end up normalized to different dates and cause rare CI flakes. Consider computing a single reference nowUTC once per test (or per StorageIntegration) and passing it into both normalization paths (e.g., correctTime(jsonData, nowUTC) and correctTimeForTrace(td, nowUTC)).
Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
| // Copyright (c) 2026 The Jaeger Authors. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package main |
There was a problem hiding this comment.
this binary can safely be removed after conversion of all fixtures
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@yurishkuro all of the copilot comments are now addressed, can you see this if the binary is correct or not |
yurishkuro
added
the
changelog:ci
jaegertracing#8044) ## Which problem is this PR solving? - Fixes a part of: jaegertracing#7050 ## Description of the changes - This init the environment to swap fixtures with v2. One of the fixtures is also swapped here. ## How was this change tested? - Unit and Integration tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully: `make lint test` ## AI Usage in this PR (choose one) See [AI Usage Policy](https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md#ai-usage-policy). - [x] **None**: No AI tools were used in creating this PR - [ ] **Light**: AI provided minor assistance (formatting, simple suggestions) - [ ] **Moderate**: AI helped with code generation or debugging specific parts - [ ] **Heavy**: AI generated most or all of the code changes --------- Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
make lint testAI Usage in this PR (choose one)
See AI Usage Policy.