Skip to content
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

Support tempo reading traces #5849

Closed
Tracked by #5850
aljesusg opened this issue Feb 16, 2023 · 4 comments · Fixed by #6518
Closed
Tracked by #5850

Support tempo reading traces #5849

aljesusg opened this issue Feb 16, 2023 · 4 comments · Fixed by #6518
Assignees
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features. sub-task Ties an issue to an epic waiting external It requires additional info to progress. For example, it can require a fix in other project.

Comments

@aljesusg
Copy link
Collaborator

Adapt Kiali to query Tempo directly

The better solution (imo) would be to add a client specific to Tempo. Looking at the code this would require a couple of changes:

The current client uses the Jaeger data model (for instance JaegerSingleTrace) but Tempo returns traces in the OpenTelemetry format (JSON or Protobuf). The proto definition: opentelemetry-proto - opentelemetry/proto/trace/v1/trace.proto.
Luckily there is already a lot of code out there to convert between OpenTelemetry and Jaeger, Zipkin. So you can convert OpenTelemetry into Jaeger in the client or opt to switch Kiali to use the OpenTelemetry format all together and do the conversion in the Jaeger client.

The search results of Jaeger and Tempo are different: while Jaeger returns a list of full traces, Tempo only returns metadata of the traces. See https://grafana.com/docs/tempo/latest/api_docs/#search
This might be a problem if you need the full trace data for visualisations like the workload detail: https://kiali.io/docs/features/tracing/#workload-detail


If not make a switch to

Jaeger => Read from GRPC
Tempo => Read from TraceQL

@aljesusg aljesusg added enhancement This is the preferred way to describe new end-to-end features. sub-task Ties an issue to an epic labels Feb 16, 2023
@aljesusg aljesusg mentioned this issue Feb 16, 2023
7 tasks
@aljesusg aljesusg added backlog Triaged Issue added to backlog waiting external It requires additional info to progress. For example, it can require a fix in other project. labels Feb 16, 2023
@jshaughn
Copy link
Collaborator

It sounds right, @aljesusg. I think we want to hide as much as possible in the lower layers so that the client/ui code doesn't care if it's Tempo, Jaeger or something else. In other words, Our API should probably return a Trace object defined for our needs, and populated via some adapter code based on the configured tracing client. Is that what you are proposing? We could let the client specify whether it needs only metadata, (the default) or the full trace. And then we'd make sure to return the full trace and hide the complexity. Or, we could just return to the client a flag like isFullTrace and the client would need to query again if it needs more detail.

@aljesusg
Copy link
Collaborator Author

It sounds right, @aljesusg. I think we want to hide as much as possible in the lower layers so that the client/ui code doesn't care if it's Tempo, Jaeger or something else. In other words, Our API should probably return a Trace object defined for our needs, and populated via some adapter code based on the configured tracing client. Is that what you are proposing? We could let the client specify whether it needs only metadata, (the default) or the full trace. And then we'd make sure to return the full trace and hide the complexity. Or, we could just return to the client a flag like isFullTrace and the client would need to query again if it needs more detail.

That's the idea. We are going to research about this but I bet that we can do it and don't change the ui

@Hronom
Copy link

Hronom commented Jul 6, 2023

I finally manage to use workaround until Kiali not get direct support for Tempo.

My setup is next:

  • AWS EKS
  • Tempo helm chart tempo-distributed 1.4.7. Deployed in tempo namespace. S3 backend enabled
  • Istio 1.18.0 with enabled strict mode for mTLS, however Istio side car enabled only for tempo-gateway
  • Kiali 1.69.0

tempo-distributed 1.4.7 helm values:

  tempo:
    memberlist:
      appProtocol: tcp

  server:
    # -- Log level. Can be set to trace, debug, info (default), warn, error, fatal, panic
    logLevel: warn
    # -- Log format. Can be set to logfmt (default) or json.
    logFormat: json

  minio:
    enabled: false

  traces:
    otlp:
      http:
        enabled: true

  gateway:
    enabled: true
    podAnnotations:
      traffic.sidecar.istio.io/includeOutboundIPRanges: ""
    extraArgs:
      log.level: warn
    verboseLogging: false
    nginxConfig:
      serverSnippet: |
        location ~ ^/jaeger-query/(.*) {
          proxy_pass       http://tempo-query-frontend.tempo.svc.cluster.local:16686/$1$is_args$args;
        }

  memcached:
    podAnnotations:
      sidecar.istio.io/inject: "false"

  ingester:
    podAnnotations:
      sidecar.istio.io/inject: "false"
    appProtocol:
      grpc: grpc

  distributor:
    podAnnotations:
      sidecar.istio.io/inject: "false"
    appProtocol:
      grpc: grpc

  compactor:
    podAnnotations:
      sidecar.istio.io/inject: "false"
    config:
      compaction:
        block_retention: 24h

  querier:
    podAnnotations:
      sidecar.istio.io/inject: "false"
    appProtocol:
      grpc: grpc

  queryFrontend:
    query:
      enabled: true
      extraArgs:
        - '--grpc-storage-plugin.configuration-file=/config/tempo-query.yaml'
      # Inspired by https://github.com/grafana/tempo/issues/434#issuecomment-754033103
      extraEnv:
        - name: TMPDIR
          value: /tmp2
      extraVolumeMounts:
        - mountPath: /tmp2
          name: tmp2
        - name: config-query
          mountPath: /config/tempo-query.yaml
          subPath: tempo-query.yaml
    extraVolumes:
      - name: tmp2
        emptyDir: { }
      - name: config-query
        configMap:
          name: tempo-config
          items:
            - key: "tempo-query.yaml"
              path: "tempo-query.yaml"
    podAnnotations:
      sidecar.istio.io/inject: "false"
    appProtocol:
      grpc: grpc

  adminApi:
    podAnnotations:
      sidecar.istio.io/inject: "false"

  serviceAccount:
    create: true
    annotations:
      eks.amazonaws.com/role-arn: "arn:aws:iam::0000000000000:role/tempo"

  storage:
    trace:
      backend: s3
      s3:
        bucket: xxx-tempo-traces
        endpoint: s3.us-east-1.amazonaws.com
        region: us-east-1
        insecure: false

Kiali CR config for tracing:

        tracing:
          enabled: true
          in_cluster_url: "http://tempo-gateway.tempo.svc.cluster.local:80/jaeger-query"
          use_grpc: false

This thing works without errors.

@caerulescens
Copy link

From @Hronom's description, I was able to get the tempo-distributed chart to work together with the kiali-operator chart. There were a few name changes from tempo-* to tempo-distributed-* to get things to work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features. sub-task Ties an issue to an epic waiting external It requires additional info to progress. For example, it can require a fix in other project.
Projects
Development

Successfully merging a pull request may close this issue.

5 participants