-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[storage][cassandra] Make v1 SpanWriter reusable for v2
#7699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7699 +/- ##
==========================================
+ Coverage 95.55% 95.57% +0.02%
==========================================
Files 307 308 +1
Lines 15395 15407 +12
==========================================
+ Hits 14710 14726 +16
+ Misses 537 534 -3
+ Partials 148 147 -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: 73 Detailed changes per snapshotsummary_metrics_snapshot_elasticsearch📊 Metrics Diff SummaryTotal Changes: 73
❌ 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_duration_milliseconds{le="+Inf",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_duration_milliseconds{le="0",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_duration_milliseconds{le="10",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_duration_milliseconds{le="100",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_duration_milliseconds{le="1000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_duration_milliseconds{le="10000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_duration_milliseconds{le="25",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
...View diff sample-rpc_server_requests_per_rpc{le="+Inf",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_requests_per_rpc{le="0",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_requests_per_rpc{le="10",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_requests_per_rpc{le="100",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_requests_per_rpc{le="1000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_requests_per_rpc{le="10000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_requests_per_rpc{le="25",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
...View diff sample-rpc_server_responses_per_rpc{le="+Inf",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_responses_per_rpc{le="0",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_responses_per_rpc{le="10",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_responses_per_rpc{le="100",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_responses_per_rpc{le="1000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_responses_per_rpc{le="10000",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
-rpc_server_responses_per_rpc{le="25",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",otel_scope_schema_url="https://opentelemetry.io/schemas/1.37.0",otel_scope_version="0.64.0",rpc_grpc_status_code="2",rpc_method="FindTraces",rpc_service="jaeger.api_v3.QueryService",rpc_system="grpc"}
... |
|
I don't follow why there are business logic changes in this PR if the objective is just refactoring to make it reusable? |
There are no business logic changes in this PR. It's just that we implemented |
For example, this is removed. How is that not a business logic change? |
That is removed from |
why? |
model.Span, that means it is used for conversion of Tags to dbmodel.TagInsertion and conversion should be in v1 writer not in core writer
|
|
filtering tags is not about conversion but about indexing or not indexing them in the db |
Yes but what we need is |
As that would need to refactor the whole |
We can change that interface to work entirely on the dbmodel input. |
@yurishkuro I was trying to refactor the |
|
it's fine |
## Which problem is this PR solving? - Fixes a part of: #6458 ## Description of the changes - This is a blocker of #7699, we need to refactor `TagFilter` to accept `dbmodel.Span` rather than `model.Span` ## How was this change tested? - Unit And 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`: `npm run lint` and `npm run test` --------- Signed-off-by: Manik Mehta <[email protected]>
Signed-off-by: Manik Mehta <[email protected]>
| StringType = "string" | ||
| BoolType = "bool" | ||
| Int64Type = "int64" | ||
| Float64Type = "float64" | ||
| BinaryType = "binary" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the reason of doing this is because we need them in writer_test and following the same we may require them in writer v2 also
…ing#7707) ## Which problem is this PR solving? - Fixes a part of: jaegertracing#6458 ## Description of the changes - This is a blocker of jaegertracing#7699, we need to refactor `TagFilter` to accept `dbmodel.Span` rather than `model.Span` ## How was this change tested? - Unit And 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`: `npm run lint` and `npm run test` --------- Signed-off-by: Manik Mehta <[email protected]> Signed-off-by: SoumyaRaikwar <[email protected]>
Which problem is this PR solving?
Description of the changes
CoreSpanWriteris seperated from v1SpanWriterHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test