refactor: use enum instead of strings for ES mappings #6068
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
f2b6b51 to
ac6de11
Compare
yurishkuro
approved these changes
Oct 7, 2024
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
|
anything im missing out on here @yurishkuro ? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6068 +/- ##
===========================================
- Coverage 96.92% 50.33% -46.59%
===========================================
Files 351 177 -174
Lines 16675 8971 -7704
===========================================
- Hits 16162 4516 -11646
- Misses 329 4024 +3695
- Partials 184 431 +247
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
yurishkuro
reviewed
Oct 8, 2024
| return ok | ||
| } | ||
|
|
||
| // stringToMappingType converts a string to a MappingType | ||
| func stringToMappingType(val string) (mappings.MappingType, error) { |
There was a problem hiding this comment.
this translation can be co-located with the MappingType definition:
func MappingTypeFromString(val string) (MappingType, error)
yurishkuro
reviewed
Oct 10, 2024
yurishkuro
reviewed
Oct 10, 2024
yurishkuro
reviewed
Oct 12, 2024
8d6ce09 to
a86d9a8
Compare
|
Had overlooked submodule while running have fixed that and merge conflicts now. |
yurishkuro
reviewed
Oct 12, 2024
yurishkuro
reviewed
Oct 13, 2024
|
please make sure all commits are signed (DCO check is failing) |
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
…rtracing#6071) Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
…egertracing#6056) Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
…in case of db issues (jaegertracing#6061) ## Which problem is this PR solving? - Resolves jaegertracing#432 - jaegertracing#432 (comment) ## Description of the changes The current [operation names table check logic](https://github.com/jaegertracing/jaeger/blob/main/plugin/storage/cassandra/spanstore/operation_names.go#L97-L99) creates `OperationNamesStorage` instance pointing to old schema table in case of issues with Cassandra. - Check for the existence of old schema table before fallback - Make `OperationNamesStorage` constructor `NewOperationsStorage` to return `error` if both old and new tables doesn't exist or both of their existence cannot be checked. Since there was no `error` returned from `NewOperationsStorage` earlier had to update the dependencies and handle the `error` accordingly. ## How was this change tested? - Ran all tests - Ran all-in-one and checked if traces are appearing ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/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 - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Arunvel Sriram <arunvelsriram@gmail.com> Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [go.opentelemetry.io/otel/exporters/otlp/otlptrace](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/exporters/prometheus](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v0.52.0` -> `v0.53.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/exporters/stdout/stdouttrace](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/metric](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/sdk](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/sdk/metric](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [go.opentelemetry.io/otel/trace](https://redirect.github.com/open-telemetry/opentelemetry-go) | `v1.30.0` -> `v1.31.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>open-telemetry/opentelemetry-go (go.opentelemetry.io/otel/exporters/otlp/otlptrace)</summary> ### [`v1.31.0`](https://redirect.github.com/open-telemetry/opentelemetry-go/releases/tag/v1.31.0): /v0.53.0/v0.7.0/v0.0.10 [Compare Source](https://redirect.github.com/open-telemetry/opentelemetry-go/compare/v1.30.0...v1.31.0) ##### Added - Add `go.opentelemetry.io/otel/sdk/metric/exemplar` package which includes `Exemplar`, `Filter`, `TraceBasedFilter`, `AlwaysOnFilter`, `HistogramReservoir`, `FixedSizeReservoir`, `Reservoir`, `Value` and `ValueType` types. These will be used for configuring the exemplar reservoir for the metrics sdk. ([#​5747](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5747), [#​5862](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5862)) - Add `WithExportBufferSize` option to log batch processor.([#​5877](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5877)) ##### Changed - Enable exemplars by default in `go.opentelemetry.io/otel/sdk/metric`. Exemplars can be disabled by setting `OTEL_METRICS_EXEMPLAR_FILTER=always_off` ([#​5778](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5778)) - `Logger.Enabled` in `go.opentelemetry.io/otel/log` now accepts a newly introduced `EnabledParameters` type instead of `Record`. ([#​5791](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5791)) - `FilterProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log/internal/x` now accepts `EnabledParameters` instead of `Record`. ([#​5791](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5791)) - The `Record` type in `go.opentelemetry.io/otel/log` is no longer comparable. ([#​5847](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5847)) - Performance improvements for the trace SDK `SetAttributes` method in `Span`. ([#​5864](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5864)) - Reduce memory allocations for the `Event` and `Link` lists in `Span`. ([#​5858](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5858)) - Performance improvements for the trace SDK `AddEvent`, `AddLink`, `RecordError` and `End` methods in `Span`. ([#​5874](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5874)) ##### Deprecated - Deprecate all examples under `go.opentelemetry.io/otel/example` as they are moved to [Contrib repository](https://redirect.github.com/open-telemetry/opentelemetry-go-contrib/tree/main/examples). ([#​5854](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5854)) ##### Fixed - The race condition for multiple `FixedSize` exemplar reservoirs identified in [#​5814](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5814) is resolved. ([#​5819](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5819)) - Fix log records duplication in case of heterogeneous resource attributes by correctly mapping each log record to it's resource and scope. ([#​5803](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5803)) - Fix timer channel drain to avoid hanging on Go 1.23. ([#​5868](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5868)) - Fix delegation for global meter providers, and panic when calling otel.SetMeterProvider. ([#​5827](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5827)) - Change the `reflect.TypeOf` to use a nil pointer to not allocate on the heap unless necessary. ([#​5827](https://redirect.github.com/open-telemetry/opentelemetry-go/issues/5827)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/jaegertracing/jaeger). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4xMTUuMSIsInVwZGF0ZWRJblZlciI6IjM4LjExNS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJjaGFuZ2Vsb2c6ZGVwZW5kZW5jaWVzIl19--> Signed-off-by: Mend Renovate <bot@renovateapp.com> Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
…aegertracing#6060) ## Which problem is this PR solving? - Part of jaegertracing#6059 ## Description of the changes - Continuation of jaegertracing#5790 - Refactors the configurations for ElasticSearch and OpenSearch to simplify the configuration by having more logical groupings ## How was this change tested? - Unit Tests and E2E Integration Tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/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 - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Jared Tan <jian.tan@daocloud.io> Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Jared Tan <jian.tan@daocloud.io> Co-authored-by: Yuri Shkuro <github@ysh.us> Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
…acing#6077) ## Which problem is this PR solving? - Towards jaegertracing#5545 ## Description of the changes - Created a new `Sanitizer` type for sanitizing traces in OTLP format - Implemented the empty service name sanitizer; I'll open a separate PR for the UTF-8 sanitizer ## How was this change tested? - Unit tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/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 - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com> Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
Signed-off-by: Saumya Shah <saumyabshah90@gmail.com>
7988268 to
d4a553d
Compare
|
@yurishkuro any way i open an another PR for this? sorry for this causing this issue from my side ! |
|
yes, feel free to open a new PR |
4 tasks
yurishkuro
added a commit
that referenced
this pull request
Oct 18, 2024
## Which problem is this PR solving? - Resolves #6067 - Follows up #6068 ## Description of the changes - Currently in `plugin/storage/es/mappings/mapping.go` we are using bare string inorder to for the file mapping. This PR refractors this approach to use enums instead of string ## How was this change tested? - running `go test` in respective files/directories ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/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 - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Saumya Shah <saumyabshah90@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
chahatsagarmain
pushed a commit
to chahatsagarmain/jaeger
that referenced
this pull request
Oct 23, 2024
## Which problem is this PR solving? - Resolves jaegertracing#6067 - Follows up jaegertracing#6068 ## Description of the changes - Currently in `plugin/storage/es/mappings/mapping.go` we are using bare string inorder to for the file mapping. This PR refractors this approach to use enums instead of string ## How was this change tested? - running `go test` in respective files/directories ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/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 - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Saumya Shah <saumyabshah90@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
Description of the changes
plugin/storage/es/mappings/mapping.gowe are using bare string inorder to for the file mapping. This PR refractors this approach to use enums instead of stringHow was this change tested?
go testChecklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test