Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

Implement Jaeger gRPC remote storage writer interface #1543

Merged
merged 3 commits into from
Aug 8, 2022

Conversation

arajkumar
Copy link
Member

@arajkumar arajkumar commented Aug 2, 2022

Sub tasks

  • Translate Jaeger model.Span to Otel ptrace.Traces
  • Write E2E test
  • Update documentation

Implements #1535.

Testing

Note: This testing strategy establishes a cyclic dependency, where the promscale exports it's internal tracing data to Jaeger collector and which would again pushes back to gRPC stroage implementation which is again promscale.

# shell 1
$ ./dist/promscale --db.name promscale --db.password promscale --db.user promscale --db.ssl-mode allow --startup.install-extensions --startup.upgrade-prerelease-extensions -telemetry.trace.jaeger-endpoint http://localhost:14268/api/traces

# shell 2
$ docker run -it -e SPAN_STORAGE_TYPE=grpc-plugin -p 14268:14268 jaegertracing/jaeger-collector:1.36 --grpc-storage.server=host.docker.internal:9202

Description

This commit implements Jaeger gRPC storage writer by using otel translator package to transforms Jager model.Span to Otel ptrace.Traces.

Signed-off-by: Arunprasad Rajkumar ar.arunprasad@gmail.com

Note: If your PR involves benchmarks, you can run the Benchmarks workflow by adding action:benchmarks label. The PR must be opened from Promscale branch so that Github actions can leave a comment comparing results against master.

Merge requirements

Please take into account the following non-code changes that you may need to make with your PR:

  • CHANGELOG entry for user-facing changes
  • Updated the relevant documentation

@@ -104,7 +104,9 @@ func GenerateRouter(apiConf *Config, promqlConf *query.Config, client *pgclient.
reloadHandler := timeHandler(metrics.HTTPRequestDuration, "/-/reload", Reload(reload, apiConf.AdminAPIEnabled))
router.Path("/-/reload").Methods(http.MethodPost).HandlerFunc(reloadHandler)

jaeger.ExtendQueryAPIs(router, client.ReadOnlyConnection(), query)
if store != nil {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is added to skip creation of Jaeger store when doing promql_endpoint_integration_test.go

@arajkumar arajkumar marked this pull request as ready for review August 3, 2022 12:42
@arajkumar arajkumar added this to the 0.14.0 milestone Aug 3, 2022
@niksajakovljevic niksajakovljevic added the Feature New features label Aug 3, 2022
@arajkumar arajkumar self-assigned this Aug 3, 2022
Copy link
Contributor

@antekresic antekresic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@niksajakovljevic niksajakovljevic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. My only concern is keeping translation part out of Ingestor.

This commit implements Jaeger gRPC storage writer by using otel translator package to transforms Jager `model.Span` to Otel `ptrace.Traces`.

Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
Since we implemented writer interface for Jaeger gRPC remote storage, it is no longer only queryable, it is also writable. Hence the same "store".

Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
@arajkumar arajkumar merged commit ca91e2f into timescale:master Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants