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

OpenTracing support #509

Closed
15 tasks done
whiskeysierra opened this issue Oct 28, 2018 · 7 comments
Closed
15 tasks done

OpenTracing support #509

whiskeysierra opened this issue Oct 28, 2018 · 7 comments
Labels
Milestone

Comments

@whiskeysierra
Copy link
Collaborator

whiskeysierra commented Oct 28, 2018

Detailed Description

Add OpenTracing integration that e.g. allows to add retry count as a tag to spans.

Context

Instrumenting the underlying Apache Http Client is a good enough solution for most, but a deeper integration allows for more sophisticated use cases (like retry count).

Possible Implementation

  • riptide-opentracing module
  • riptide-spring-boot-autoconfigure
    • plugin needs to be registered directly after/before timeout plugin (auto configuration)
    • instrument executor and scheduler using opentracing-concurrent
    • instrument Apache Http Client using jaeger-apachehttpclient without dependency to jaeger!
      • Propagate tags?
    • Set client id as tag?
    • Degree of configurability?

Example Configuration

riptide:
  defaults:
    tracing:
      enabled: true
      tags:
        account: ${CDP_TARGET_INFRASTRUCTURE_ACCOUNT}
        zone: ${CDP_TARGET_REGION}
        artifact_version: ${CDP_BUILD_VERSION}
        deployment_id: ${CDP_DEPLOYMENT_ID}
      propagate-flow-id: true
        
  clients:
    exchange-rate:
      tracing:
        tags:
          peer.service: exchange-rate-service
        propagate-flow-id: false

Your Environment

  • Version used:
  • Link to your project:
@tkrop
Copy link
Member

tkrop commented Oct 29, 2018

I'm not sure about the value of retry count - OpenTracing needs aggregate over spans anyhow, making a retry count optical visible. However, a consistent higher level operation name and an idempotency key to distinguish different requests could be a value. Riptide does not provide an operation name, does it? Anyhow, it should be easy to calculate a consistent operation name from the path template - something that is difficult from the HTTP client itself.

@whiskeysierra
Copy link
Collaborator Author

OpenTracing needs aggregate over spans anyhow, making a retry count optical visible

See also https://github.bus.zalan.do/SRE/opentracing/issues/6.
Visible yes, but indistinguishable from sequential requests to the same endpoint.

Anyhow, it should be easy to calculate a consistent operation name from the path template

I can imagine proving something for simplistic templates, but for more complex ones it might not be as easy to generate a consistent operation name.

@tkrop
Copy link
Member

tkrop commented Oct 29, 2018

See also https://github.bus.zalan.do/SRE/opentracing/issues/6.

Interesting. Overlooked it. How do you correlate two spans that are retries, when you concurrently issue other requests to the same endpoint - as we did long time ago in our project? May be this issue solves only have the problem. Should all requests with retries be wrapped into an enclosing span for this remote request?

@whiskeysierra
Copy link
Collaborator Author

How do you correlate two spans that are retries, when you concurrently issue other requests to the same endpoint

Either by doing this:

retries be wrapped into an enclosing span

or

an idempotency key to distinguish different requests

@whiskeysierra whiskeysierra modified the milestone: 3.0.0 Nov 15, 2018
@whiskeysierra whiskeysierra added this to the 3.0.0 milestone Feb 6, 2019
@whiskeysierra
Copy link
Collaborator Author

Requires zalando/opentracing-toolbox#204

@whiskeysierra
Copy link
Collaborator Author

whiskeysierra commented Feb 22, 2019

Instead of using an interceptor for Apache Http Client, maybe we can just register our new OpenTracingPlugin twice. Once for enclosing span and a second time after the failsafe plugin in case we have retries enabled.

@whiskeysierra whiskeysierra pinned this issue Feb 23, 2019
@whiskeysierra
Copy link
Collaborator Author

#450 would introduce a more fine-grained approach to error handling which might change which of those exceptions we treat as an error (error tag set to true). For example a response being translated into an exception within the routing tree might not be what OpenTracing considers an error.

@whiskeysierra whiskeysierra mentioned this issue Feb 24, 2019
6 tasks
@whiskeysierra whiskeysierra modified the milestones: 3.0.0, 3.1.0 Mar 2, 2019
@whiskeysierra whiskeysierra modified the milestones: 3.1.0, 3.0.0 Mar 14, 2019
@whiskeysierra whiskeysierra unpinned this issue Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants