[ES] Add `Scope` field to DB Span #7039

Manik2708 · 2025-04-16T17:21:03Z

Which problem is this PR solving?

Fixes a part of: [Feature]: Upgrade Elasticsearch Storage to v2 API #7034

Description of the changes

Currently it is impossible to record scope information completely as there is no way to store scope attributes in ES DB Span. Therefore it is necessary to add a scope field in db span.

How was this change tested?

Unit And integration tests

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: npm run lint and npm run test

Signed-off-by: Manik2708 <[email protected]>

internal/storage/v1/elasticsearch/mappings/mapping_test.go

codecov · 2025-04-16T17:28:25Z

Codecov Report

❌ Patch coverage is 95.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.09%. Comparing base (734e1be) to head (3ddaf4b).
⚠️ Report is 280 commits behind head on main.

Files with missing lines	Patch %	Lines
...torage/v2/elasticsearch/tracestore/from_dbmodel.go	92.68%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7039      +/-   ##
==========================================
+ Coverage   96.04%   96.09%   +0.04%     
==========================================
  Files         352      352              
  Lines       20821    20820       -1     
==========================================
+ Hits        19998    20007       +9     
+ Misses        620      613       -7     
+ Partials      203      200       -3

Flag	Coverage Δ
badger_v1	`9.96% <0.00%> (-0.01%)`	⬇️
badger_v2	`2.07% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-v1-manual	`14.99% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-v2-auto	`2.06% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-v2-manual	`2.06% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-v1-manual	`14.99% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-v2-auto	`2.06% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-v2-manual	`2.06% <0.00%> (-0.01%)`	⬇️
elasticsearch-6.x-v1	`19.98% <100.00%> (+0.03%)`	⬆️
elasticsearch-7.x-v1	`20.06% <100.00%> (+0.03%)`	⬆️
elasticsearch-8.x-v1	`20.23% <100.00%> (+0.03%)`	⬆️
elasticsearch-8.x-v2	`2.07% <0.00%> (-0.01%)`	⬇️
grpc_v1	`11.51% <0.00%> (-0.01%)`	⬇️
grpc_v2	`9.14% <0.00%> (-0.01%)`	⬇️
kafka-3.x-v1	`10.24% <0.00%> (-0.01%)`	⬇️
kafka-3.x-v2	`2.07% <0.00%> (-0.01%)`	⬇️
memory_v2	`2.07% <0.00%> (-0.01%)`	⬇️
opensearch-1.x-v1	`20.11% <100.00%> (+0.03%)`	⬆️
opensearch-2.x-v1	`20.11% <100.00%> (+0.03%)`	⬆️
opensearch-2.x-v2	`2.07% <0.00%> (-0.01%)`	⬇️
tailsampling-processor	`0.56% <0.00%> (-0.01%)`	⬇️
unittests	`94.85% <95.00%> (+0.04%)`	⬆️

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.

yurishkuro · 2025-04-16T17:32:14Z

internal/storage/elasticsearch/dbmodel/model.go

+	Name    string         `json:"name,omitempty"`
+	Version string         `json:"version,omitempty"`
+	Tags    []KeyValue     `json:"tags,omitempty"`
+	Tag     map[string]any `json:"tag,omitempty"`


why can't we add another field TagType to capture the type of value and avoid data loss? We'd want to do that for all 3 flavors of Tag field

Will add it in the DotReplacement PR as tag map handling is taking place there!

internal/storage/v1/elasticsearch/mappings/fixtures/jaeger-span-6.json

internal/storage/v1/elasticsearch/spanstore/reader.go

yurishkuro · 2025-04-16T17:37:46Z

internal/storage/v1/elasticsearch/spanstore/reader.go

 	operationNameField     = "operationName"
 	objectTagsField        = "tag"
 	objectProcessTagsField = "process.tag"
+	objectScopeTagsField   = "scope.tag"


how come this is the only change in the reader? Doesn't it need to alter queries sent to ES to look in the new fields?

And how will it work if the ES template doesn't have those fields (legacy data)? Will the query fail or just not find anything?

how come this is the only change in the reader? Doesn't it need to alter queries sent to ES to look in the new fields?

The changes are automatically captured by the array to reflect in the query! Query looks for all the tags therefore we just need to add it in the nestedTagFieldList and objectTagFieldList

And how will it work if the ES template doesn't have those fields (legacy data)? Will the query fail or just not find anything?

For nested field query it will fail as we need to inform to ES about nested fields to make nested query success and for object fields, it will return nothing

how come this is the only change in the reader? Doesn't it need to alter queries sent to ES to look in the new fields?

The changes are automatically captured by the array to reflect in the query! Query looks for all the tags therefore we just need to add it in the nestedTagFieldList and objectTagFieldList

And how will it work if the ES template doesn't have those fields (legacy data)? Will the query fail or just not find anything?

For nested field query it will fail as we need to inform to ES about nested fields to make nested query success and for object fields, it will return nothing

My bad! It will fail only the nested query, nothing will happen for object (everything will be fine)!

We can't have queries against old data fail. What's the solution?

I have thought 2 step solution. We should understand that the only thing we need to ensure is: Mappings are up-to-date.

The users which have enabled CreateIndexTemplates: We can introduce a feature gate to automatically update the mappings.

The users which have disabled CreateIndexTemplates: We can have an update log to inform them the changes they might have to make in the mappings before updating jaeger.

Can we detect automatically if the mapping is missing something? Don't we allow users to make manual changes to the mappings too? You need to think through possible permutations of starting conditions and have a path for each of them.

I just want to know a thing, when users will take the update, the ES Factory will be re-initialized, right? If yes then I think new templates will automatically be created if user has enabled CreateIndexTemplates. Please see this code:

jaeger/internal/storage/v1/elasticsearch/factory.go

Lines 248 to 256 in 40e1ff3

if cfg.CreateIndexTemplates && !cfg.UseILM {

mappingBuilder := mappingBuilderFromConfig(cfg)

spanMapping, serviceMapping, err := mappingBuilder.GetSpanServiceMappings()

if err != nil {

return nil, err

}

if err := writer.CreateTemplates(spanMapping, serviceMapping, cfg.Indices.IndexPrefix); err != nil {

return nil, err

}

Internally we use *elastic.IndicesPutTemplateService which replaces the mapping if it already exists. For the users who have turned off CreateIndexTemplates we can inform them to make the necessary changes before updating jaeger.

Don't we allow users to make manual changes to the mappings too?

Can you please explain, how users can do this? I can understand 2 cases around this:

Users who have enabled CreateIndexTemplates and manually edited the mappings later, the new mappings will be overrided on the old mappings automatically.

Users who have disabled CreateIndexTemplates, as said above we need to inform them about the necessay changes!

So you're saying writer.CreateTemplates will always override the mappings on collector restart? Then yes we'd just need to provide migration instructions for users who have the setting set to manual creation.

Yes, but firstly I am thinking to test inserting some old data and then restarting jaeger and then see whether the queries are running fine or not. Have to make some changes in bash scripts for this locally. Will get back with enough evidences and then I think work on migration guide!

Sounds good. All of that should be documented in the PR description, which we can label as breaking change to call people's attention.

Signed-off-by: Manik2708 <[email protected]>

Manik2708 · 2025-04-19T09:43:33Z

@yurishkuro Good news! Now queries are not crashing no matter whether mappings are updated or not. This has become possible by using IgnoreUnmapped of nested queries. We can test this (I did) by copying only the reader and model changes to a new branch and running integration tests. They are passing without any error! Now taking about mapping changes then yes they are updated by jaeger restart. I have tested this by following steps:

Checkout to the main branch and modify the bash script of E2E tests of ES so that storage is not teared down after the tests.
Modify the tests so that indices and mappings are not cleared (Comment out cleanup functions)
Run the integration tests!
Curl the ES end-point to get the mappings.

curl -X GET "http://localhost:9200/_mapping?pretty"

Now checkout to this branch
Modify the integration tests by commenting out all the spanstore tests except those of FindTraces
When these tests are running, curl the ES endpoint to see the mappings (you will see scope is added to the mapping).

yurishkuro · 2025-04-19T18:14:48Z

Good news! Now queries are not crashing no matter whether mappings are updated or not. This has become possible by using IgnoreUnmapped of nested queries.

Indeed good news. However, the scope attributes are not being tested in the e2e tests today, we need to add that to the suite. It may require some refactoring because the existing e2e tests work off the v1 model which cannot adequately represent scope attributes

jaeger/internal/storage/integration/integration.go

Line 348 in 5873d1f

s.writeTrace(t, trace)

We need to add the ability to read fixtures in OTLP and use v2 Storage API to write from the test (in the v2 e2e tests these writes are already automatically upgraded to v2 Storage API, but since the input is v1 we still can't test the scope attributes). This could be a separate prequel PR - ideally we would upgrade all fixtures to OTLP first, then it will be easier to extend fixtures to include resource and scope attributes.

yurishkuro · 2025-04-19T18:23:19Z

We need to add the ability to read fixtures in OTLP and use v2 Storage API to write from the test

NB: this doesn't need to be a big bang, we can just upgrade the path to write via V2 API first and remove unnecessary translations on the way ... #7050

Manik2708 · 2025-04-19T20:17:44Z

We need to add the ability to read fixtures in OTLP and use v2 Storage API to write from the test

NB: this doesn't need to be a big bang, we can just upgrade the path to write via V2 API first and remove unnecessary translations on the way ... #7050

@yurishkuro Then before that I think we need to implement the factory for ES, or should we directly use the v2 writer? I am thinking to first refactor the v1 factory, then implement the v2 factory and then finally upgrade the integration tests! Does this look fine?

yurishkuro · 2025-04-19T22:35:41Z

I think we need to implement the factory for ES

well, refactoring e2e tests is a good chunk of changes by itself. It might be easier to finish v2 memory storage first and use it as a guinea pig, rather than introducing dependency on ES just for the e2e tests upgrade. It's also much easier/faster to run e2e tests for memory storage.

Manik2708 · 2025-04-20T18:50:23Z

I think we need to implement the factory for ES

well, refactoring e2e tests is a good chunk of changes by itself. It might be easier to finish v2 memory storage first and use it as a guinea pig, rather than introducing dependency on ES just for the e2e tests upgrade. It's also much easier/faster to run e2e tests for memory storage.

Have a doubt over this! Currently we have implemented v2 APIs for two backends: ElasticSearch and Memory. The problem is both of them would give different traces (only structure wise), although no information loss is there but take an example, every Span is appended to a different resource span in ES but this is not so in memory. So how should we do testing now?

yurishkuro · 2025-04-20T18:52:12Z

v2 API is also implemented for GRPC storage.

yurishkuro · 2025-04-20T18:53:36Z

would give different traces (only structure wise)

we can apply some normalization function in the test before comparing with expected fixture, e.g. to split the whole payload such that each span has its own copy of resource & scope objects (even if they are duplicated).

github-actions · 2025-07-28T02:16:43Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You may re-open it if you need more time.

yurishkuro · 2025-07-28T02:59:08Z

@Manik2708 is this still needed?

Manik2708 · 2025-07-28T03:44:46Z

@Manik2708 is this still needed?

@yurishkuro Extremely sorry for not looking into this. But this can be completed once refactoring of e2e tests is done and we could add integration tests related to scope

github-actions · 2025-10-06T02:07:32Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You may re-open it if you need more time.

[ES] Add Scope field to DB Span

506a989

Signed-off-by: Manik2708 <[email protected]>

Manik2708 requested a review from a team as a code owner April 16, 2025 17:21

Manik2708 requested a review from mahadzaryab1 April 16, 2025 17:21

Merge branch 'main' into scope

c81e5d4

dosubot bot added the storage/elasticsearch label Apr 16, 2025

Manik2708 commented Apr 16, 2025

View reviewed changes

internal/storage/v1/elasticsearch/mappings/mapping_test.go Show resolved Hide resolved

yurishkuro reviewed Apr 16, 2025

View reviewed changes

internal/storage/v1/elasticsearch/mappings/fixtures/jaeger-span-6.json Outdated Show resolved Hide resolved

yurishkuro reviewed Apr 16, 2025

View reviewed changes

internal/storage/v1/elasticsearch/spanstore/reader.go Show resolved Hide resolved

yurishkuro reviewed Apr 16, 2025

View reviewed changes

yurishkuro added the changelog:exprimental Change to an experimental part of the code label Apr 18, 2025

Manik2708 added 3 commits April 19, 2025 14:44

cleanup

9352fb3

Signed-off-by: Manik2708 <[email protected]>

Merge branch 'main' into scope

83caa9f

cleanup

3ddaf4b

Signed-off-by: Manik2708 <[email protected]>

Manik2708 mentioned this pull request Jul 1, 2025

[Feature]: Upgrade Elasticsearch Storage to v2 API #7034

Open

8 tasks

github-actions bot added the stale The issue/PR has become stale and may be auto-closed label Jul 28, 2025

github-actions bot removed the stale The issue/PR has become stale and may be auto-closed label Aug 4, 2025

github-actions bot added the stale The issue/PR has become stale and may be auto-closed label Oct 6, 2025

	if cfg.CreateIndexTemplates && !cfg.UseILM {
	mappingBuilder := mappingBuilderFromConfig(cfg)
	spanMapping, serviceMapping, err := mappingBuilder.GetSpanServiceMappings()
	if err != nil {
	return nil, err
	}
	if err := writer.CreateTemplates(spanMapping, serviceMapping, cfg.Indices.IndexPrefix); err != nil {
	return nil, err
	}

[ES] Add Scope field to DB Span #7039

Are you sure you want to change the base?

[ES] Add Scope field to DB Span #7039

Uh oh!

Conversation

Manik2708 commented Apr 16, 2025

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manik2708 Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurishkuro Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manik2708 commented Apr 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurishkuro commented Apr 19, 2025

Uh oh!

yurishkuro commented Apr 19, 2025

Uh oh!

Manik2708 commented Apr 19, 2025

Uh oh!

yurishkuro commented Apr 19, 2025

Uh oh!

Manik2708 commented Apr 20, 2025

Uh oh!

yurishkuro commented Apr 20, 2025

Uh oh!

yurishkuro commented Apr 20, 2025

Uh oh!

github-actions bot commented Jul 28, 2025

Uh oh!

yurishkuro commented Jul 28, 2025

Uh oh!

Manik2708 commented Jul 28, 2025

Uh oh!

github-actions bot commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[ES] Add `Scope` field to DB Span #7039

[ES] Add `Scope` field to DB Span #7039

codecov bot commented Apr 16, 2025 •

edited

Loading

Manik2708 Apr 16, 2025 •

edited

Loading

yurishkuro Apr 18, 2025 •

edited

Loading

Manik2708 commented Apr 19, 2025 •

edited

Loading