[elasticsearch] Move core span reader/writer to v2/elasticsearch/tracestore/core by yurishkuro · Pull Request #8328 · jaegertracing/jaeger

yurishkuro · 2026-04-09T02:11:39Z

Move internal/storage/v1/elasticsearch/spanstore/ to internal/storage/v2/elasticsearch/tracestore/core/, renaming package from 'spanstore' to 'core'
Move internal/storage/elasticsearch/dbmodel/ to internal/storage/v2/elasticsearch/tracestore/core/dbmodel/
Extract time-range index functions into new internal/storage/elasticsearch/indices/ package (shared with metrics store)
Rename CoreSpanReader/CoreSpanWriter interfaces to Reader/Writer to avoid stuttering as core.CoreSpanReader
Update .mockery.yaml and regenerate mocks

AI Usage in this PR (choose one)

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

…estore/core - Move internal/storage/v1/elasticsearch/spanstore/ to internal/storage/v2/elasticsearch/tracestore/core/, renaming package from 'spanstore' to 'core' - Move internal/storage/elasticsearch/dbmodel/ to internal/storage/v2/elasticsearch/tracestore/core/dbmodel/ - Extract time-range index functions into new internal/storage/elasticsearch/indices/ package (shared with metrics store) - Rename CoreSpanReader/CoreSpanWriter interfaces to Reader/Writer to avoid stuttering as core.CoreSpanReader - Update .mockery.yaml and regenerate mocks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Copilot

Pull request overview

This PR refactors the Elasticsearch trace storage implementation by relocating the low-level (DB model) reader/writer into internal/storage/v2/elasticsearch/tracestore/core, extracting shared time-range index selection logic into internal/storage/elasticsearch/indices, and updating interface names and mocks accordingly.

Changes:

Move/rename the ES “core” span reader/writer into v2/elasticsearch/tracestore/core and rename CoreSpanReader/CoreSpanWriter → Reader/Writer.
Move Elasticsearch trace DB model types under v2/elasticsearch/tracestore/core/dbmodel and update tracestore adapters/tests to use the new path.
Introduce internal/storage/elasticsearch/indices for shared time-range index naming logic and update both tracestore + metricstore to use it; regenerate mockery mocks.

Reviewed changes

Copilot reviewed 28 out of 37 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/storage/v2/elasticsearch/tracestore/writer.go	Switch tracestore writer wrapper to use `core` package writer + params.
internal/storage/v2/elasticsearch/tracestore/writer_test.go	Update tests to use new `core/mocks` Writer mock and params types.
internal/storage/v2/elasticsearch/tracestore/to_dbmodel.go	Update dbmodel import to new `core/dbmodel` location.
internal/storage/v2/elasticsearch/tracestore/to_dbmodel_test.go	Update dbmodel import to new `core/dbmodel` location.
internal/storage/v2/elasticsearch/tracestore/reader.go	Switch tracestore reader wrapper to use `core` reader + params and new dbmodel.
internal/storage/v2/elasticsearch/tracestore/reader_test.go	Update tests to use new `core` reader/dbmodel imports and mocks.
internal/storage/v2/elasticsearch/tracestore/ids.go	Update dbmodel import to new `core/dbmodel` location.
internal/storage/v2/elasticsearch/tracestore/from_dbmodel.go	Update dbmodel import to new `core/dbmodel` location.
internal/storage/v2/elasticsearch/tracestore/from_dbmodel_test.go	Update dbmodel import to new `core/dbmodel` location.
internal/storage/v2/elasticsearch/tracestore/core/writer.go	Rename package to `core`, use shared `indices.IndexWithDate`, and rename writer interface to `Writer`.
internal/storage/v2/elasticsearch/tracestore/core/writer_test.go	Update tests to use `indices.IndexWithDate` and new dbmodel import path.
internal/storage/v2/elasticsearch/tracestore/core/service_operation.go	Rename package to `core` and update dbmodel import path.
internal/storage/v2/elasticsearch/tracestore/core/service_operation_test.go	Rename package to `core` and update dbmodel import path.
internal/storage/v2/elasticsearch/tracestore/core/reader.go	Rename package to `core` and replace inline time-range index helpers with shared `indices` package.
internal/storage/v2/elasticsearch/tracestore/core/reader_test.go	Update tests to use `indices.IndexWithDate` and align test naming with new helper location.
internal/storage/v2/elasticsearch/tracestore/core/reader_iface.go	Rename package to `core`, rename interface `CoreSpanReader` → `Reader`, and update dbmodel import.
internal/storage/v2/elasticsearch/tracestore/core/package_test.go	Rename package to `core` for leak-check test harness.
internal/storage/v2/elasticsearch/tracestore/core/mocks/mocks.go	Add regenerated mockery mocks for `core.Reader` and `core.Writer`.
internal/storage/v2/elasticsearch/tracestore/core/json_span_compare_test.go	Rename package to `core` and update dbmodel import path.
internal/storage/v2/elasticsearch/tracestore/core/fixtures/query_01.json	Add query fixture under new `core` location.
internal/storage/v2/elasticsearch/tracestore/core/fixtures/query_02.json	Add query fixture under new `core` location.
internal/storage/v2/elasticsearch/tracestore/core/fixtures/query_03.json	Add query fixture under new `core` location.
internal/storage/v2/elasticsearch/tracestore/core/fixtures/es_01.json	Add ES span JSON fixture under new `core` location.
internal/storage/v2/elasticsearch/tracestore/core/fixtures/domain_01.json	Add domain span JSON fixture under new `core` location.
internal/storage/v2/elasticsearch/tracestore/core/dbmodel/package_test.go	Add leak-check harness for new dbmodel package.
internal/storage/v2/elasticsearch/tracestore/core/dbmodel/model.go	Add DB model types under `core/dbmodel`.
internal/storage/v2/elasticsearch/tracestore/core/dbmodel/dot_replacer.go	Add dot-replacement helper used by reader tag field handling.
internal/storage/v2/elasticsearch/tracestore/core/dbmodel/dot_replacer_test.go	Add unit test for dot-replacement helper.
internal/storage/v1/elasticsearch/spanstore/mocks/mocks.go	Remove v1 spanstore mocks after relocating core interfaces/mocks.
internal/storage/v1/elasticsearch/spanstore/index_utils.go	Remove v1-only `indexWithDate` helper (now in shared `indices`).
internal/storage/v1/elasticsearch/factory.go	Point v1 Elasticsearch factory span params/types at relocated v2 `core` implementation.
internal/storage/v1/elasticsearch/factory_test.go	Update tests to use `core` writer/params and dbmodel types from new location.
internal/storage/metricstore/elasticsearch/query_builder.go	Use shared `indices.TimeRangeIndexFn` utilities instead of v1 spanstore helpers.
internal/storage/elasticsearch/indices/time_range.go	New shared package for time-range index computation + logging wrapper + `IndexWithDate`.
internal/storage/elasticsearch/indices/time_range_test.go	Add unit tests for shared time-range index utilities.
internal/storage/elasticsearch/indices/package_test.go	Add leak-check harness for new `indices` package.
.mockery.yaml	Update mockery config to generate mocks for `core.Reader`/`core.Writer`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/storage/v2/elasticsearch/tracestore/reader.go

internal/storage/metricstore/elasticsearch/query_builder.go

codecov · 2026-04-09T02:28:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (2cd60ac) to head (b6ee078).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8328      +/-   ##
==========================================
- Coverage   95.61%   95.60%   -0.02%     
==========================================
  Files         314      314              
  Lines       16455    16455              
==========================================
- Hits        15734    15732       -2     
- Misses        568      570       +2     
  Partials      153      153

Flag	Coverage Δ
badger_direct	`9.33% <0.00%> (ø)`
badger_e2e	`1.08% <0.00%> (ø)`
cassandra-4.x-direct-manual	`13.66% <0.00%> (ø)`
cassandra-4.x-e2e-auto	`1.07% <0.00%> (ø)`
cassandra-4.x-e2e-manual	`1.07% <0.00%> (ø)`
cassandra-5.x-direct-manual	`13.66% <0.00%> (ø)`
cassandra-5.x-e2e-auto	`1.07% <0.00%> (ø)`
cassandra-5.x-e2e-manual	`1.07% <0.00%> (ø)`
clickhouse	`1.20% <0.00%> (ø)`
elasticsearch-6.x-direct	`17.51% <80.70%> (ø)`
elasticsearch-7.x-direct	`17.54% <80.70%> (ø)`
elasticsearch-8.x-direct	`17.69% <80.70%> (ø)`
elasticsearch-8.x-e2e	`1.08% <0.00%> (ø)`
elasticsearch-9.x-e2e	`1.08% <0.00%> (ø)`
grpc_direct	`8.11% <0.00%> (ø)`
grpc_e2e	`1.08% <0.00%> (ø)`
kafka-3.x-v2	`1.15% <0.00%> (+0.07%)`	⬆️
memory_v2	`1.08% <0.00%> (ø)`
opensearch-1.x-direct	`17.58% <80.70%> (ø)`
opensearch-2.x-direct	`17.58% <80.70%> (ø)`
opensearch-2.x-e2e	`1.08% <0.00%> (ø)`
opensearch-3.x-e2e	`1.08% <0.00%> (ø)`
query	`1.08% <0.00%> (ø)`
tailsampling-processor	`0.54% <0.00%> (ø)`
unittests	`94.25% <100.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Yuri Shkuro <github@ysh.us>

Use assert.Contains/NotContains instead of assert.True/False with strings.Contains. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Copilot

Pull request overview

Copilot reviewed 29 out of 38 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T04:01:12Z

internal/storage/metricstore/elasticsearch/query_builder_test.go

+	var requestPath string
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requestPath = r.URL.Path
 		w.Header().Set("Content-Type", "application/json")


TestExecute writes to requestPath inside the httptest server handler and reads it later in the test without synchronization. This introduces a potential data race under -race. Use a channel (buffered size 1), sync/atomic, or a mutex to pass the observed request path back to the test goroutine safely.

Copilot AI review requested due to automatic review settings April 9, 2026 02:11

yurishkuro added the changelog:refactoring Internal code refactoring without functional changes label Apr 9, 2026

yurishkuro requested a review from a team as a code owner April 9, 2026 02:11

dosubot bot added area/storage storage/elasticsearch labels Apr 9, 2026

Copilot started reviewing on behalf of yurishkuro April 9, 2026 02:12 View session

jaegertracingbot approved these changes Apr 9, 2026

View reviewed changes

Copilot AI reviewed Apr 9, 2026

View reviewed changes

internal/storage/v2/elasticsearch/tracestore/reader.go Outdated Show resolved Hide resolved

internal/storage/metricstore/elasticsearch/query_builder.go Show resolved Hide resolved

yurishkuro and others added 2 commits April 8, 2026 22:44

fix

01a8c1d

Signed-off-by: Yuri Shkuro <github@ysh.us>

[elasticsearch] Fix testifylint warnings in query_builder_test.go

b6ee078

Use assert.Contains/NotContains instead of assert.True/False with strings.Contains. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Copilot AI review requested due to automatic review settings April 9, 2026 03:56

Copilot started reviewing on behalf of yurishkuro April 9, 2026 03:57 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

yurishkuro merged commit 7f35cfc into jaegertracing:main Apr 9, 2026
75 checks passed

yurishkuro deleted the refactor-es branch April 9, 2026 05:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[elasticsearch] Move core span reader/writer to v2/elasticsearch/tracestore/core#8328

[elasticsearch] Move core span reader/writer to v2/elasticsearch/tracestore/core#8328
yurishkuro merged 3 commits intojaegertracing:mainfrom
yurishkuro:refactor-es

yurishkuro commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Apr 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yurishkuro commented Apr 9, 2026

AI Usage in this PR (choose one)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Apr 9, 2026 •

edited

Loading