[es] Add missing scope and link fields to index templates#8643
Conversation
Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
|
@mahadzaryab1 One of the parts for the per-requisite, please review! |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Jaeger span Elasticsearch mappings (and fixtures) to support additional span context and scope-tag fields across ES 6/7/8.
Changes:
- Add
traceState,flags, and nestedtagsfields under the existing nested structure (e.g., references/links). - Add
scopeTag.*dynamic-template mapping plusscopeTag/scopeTagsfields to the top-level properties. - Mirror the same mapping updates into the mapping fixtures for each ES version.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/storage/v1/elasticsearch/mappings/jaeger-span-8.json | Adds scope tag dynamic template + new span context fields and scopeTag/scopeTags properties for ES8 mapping. |
| internal/storage/v1/elasticsearch/mappings/jaeger-span-7.json | Same mapping additions for ES7. |
| internal/storage/v1/elasticsearch/mappings/jaeger-span-6.json | Same mapping additions for ES6 (with slightly different field ordering). |
| internal/storage/v1/elasticsearch/mappings/fixtures/jaeger-span-8.json | Keeps ES8 fixture mapping in sync with the ES8 mapping changes. |
| internal/storage/v1/elasticsearch/mappings/fixtures/jaeger-span-7.json | Keeps ES7 fixture mapping in sync with the ES7 mapping changes. |
| internal/storage/v1/elasticsearch/mappings/fixtures/jaeger-span-6.json | Keeps ES6 fixture mapping in sync with the ES6 mapping changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| { | ||
| "scope_tags_map": { | ||
| "mapping": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "path_match": "scopeTag.*" | ||
| } |
There was a problem hiding this comment.
have we received such issue before? because the current pattern is just an extension of old pattern of span and process tags
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "path_match": "scopeTag.*" |
There was a problem hiding this comment.
writer converts all the values to string before sending it to db although I thought of changing this but then I suspect that it might break the reader therefore I kept it same (reader is always expecting string values to be matched)
| "flags":{ | ||
| "type":"integer" | ||
| }, | ||
| "traceState":{ | ||
| "type":"keyword", | ||
| "ignore_above":256 | ||
| }, |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8643 +/- ##
==========================================
- Coverage 96.57% 96.54% -0.04%
==========================================
Files 334 334
Lines 17721 17721
==========================================
- Hits 17114 17108 -6
- Misses 455 459 +4
- Partials 152 154 +2
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:
|
CI Summary ReportMetrics ComparisonView changed metricsFor label-level diff details, open the CI run and expand the "Compare metrics and generate summary" step logs. metrics_snapshot_elasticsearch_9.x_e2e — ⬇️ download diff
Code Coverage✅ Coverage 97.3% (baseline 97.3%) ➡️ View CI run | View publish logs |
| "type":"keyword", | ||
| "ignore_above":256 | ||
| }, | ||
| "tags":{ |
There was a problem hiding this comment.
what is this for? Why didn't we need it before?
There was a problem hiding this comment.
otel links have flags, tracestate and attributes which were earlier being skipped as dbmodel didn't have space to store them. adding this will make the dbmodel parallel to otel model
…ing#8643) ## Which problem is this PR solving? - Pr-requisite of: jaegertracing#8473 ## Description of the changes - Required changes in index templates as one of the parts of the PR ## How was this change tested? - Unit 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> Signed-off-by: Victor Chidera Obiezue <obiezuechidera@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.