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

Add OpenTracing/Census/Telemetry support for OPA API's #1469

Closed
patrick-east opened this issue Jun 4, 2019 · 10 comments · Fixed by #4029
Closed

Add OpenTracing/Census/Telemetry support for OPA API's #1469

patrick-east opened this issue Jun 4, 2019 · 10 comments · Fixed by #4029

Comments

@patrick-east
Copy link
Contributor

Some history on the drive for this in #1451

Basically we want to allow for systems to do tracing of requests that flow through OPA for better system-wide visibility.

As-is a user of OPA can pass in arbitrary inputs and via the policy/query arbitrary outputs for external request/trace ID's. OPA will also generate a unique decision ID as it does evaluations.

It would be ideal if we supported using/forwarding trace contexts via some standard spec like OpenTracing, OpenCensus, in the future OpenTelemetry, etc. On its own this can give some powerful debug/tracing features for OPA internally, but even more powerful when integrated with larger systems that are utilizing these tracing frameworks.

The changes required would be to use something like the OpenTracing golang library in OPA (we could maybe avoid vendoring something too, not sure how much code we really need). Then on the API server we need to extract the span context, or make a new one if it wasn't specified. Then pass it around with the context we already use everywhere internally. We could go crazy with spans and tracing stuff in OPA but as a starting point just having entry/exit to the REST API is probably enough for most use-cases.

Reference:
https://opentelemetry.io/
https://opentracing.io/
https://opentracing.io/guides/golang/inject-extract/
https://opentracing.io/guides/golang/

@dhenneke
Copy link

Are there any plans to implement this in the near future?

@patrick-east
Copy link
Contributor Author

I don't think anyone is working on it right now. It's open for community contributions.

@cwlowder
Copy link

cwlowder commented Mar 9, 2021

I don't think anyone is working on it right now. It's open for community contributions.

Can someone add the help wanted label to improve visibility?

@anderseknert
Copy link
Member

Added @cwlowder .. "voting" with a thumbs up on the original issue comment is also useful to help gauge interest.

@srenatus
Copy link
Contributor

Some questions we should discuss here, that came up related to #4029. (cc @rvalkenaers)

  1. What is our desired behaviour when OPA is embedded as a library? And how does it inform our code layout?
  2. When combined with the http.send built-in, what information should the spans it emits include?
    • Does it all get propagated through context.Context as present in bctx.ctx?

@srenatus
Copy link
Contributor

There's a bit of an analogy with the prometheus endpoint -- when OPA is embedded as a Go library, how does it's prometheus metrics collection interact with the embedding code's?

@tsandall
Copy link
Member

My 2c. is that we should avoid impacting library users as much as possible. Concretely, I would take this to mean:

  1. github.com/open-policy-agent/opa/rego package users must be unaffected
  2. github.com/open-policy-agent/opa/sdk package users must be unaffected

Other users, e.g., plugin implementors or other library users that rely on the server package directly may be affected but if we can reduce the impact, we should try.

Re: (1) and (2): if we want to provide a way for library users to opt-in, that's fine.

@srenatus
Copy link
Contributor

That sounds reasonable! Let's aim for adding this to the server first while keeping the rest unaffected. Adding some kind of integration functionality for embeddings would be a decent follow-up. (Given there's interest.)

@tsandall tsandall removed this from Backlog in Open Policy Agent Dec 2, 2021
rvalkenaers pushed a commit to rvalkenaers/opa that referenced this issue Dec 13, 2021
This commit implements tracing using the net/http automatic instrumentation wrappers on the server and topdown/http packages.

Fixes open-policy-agent#1469
Signed-off-by: Rien Valkenaers <rien.valkenaers@gmail.com>
Co-Authored-By: Stephan Renatus <stephan@styra.com>
srenatus pushed a commit that referenced this issue Dec 13, 2021
This commit implements tracing using the net/http automatic
instrumentation wrappers on the server and topdown/http packages.

Fixes #1469

Signed-off-by: Rien Valkenaers <rien.valkenaers@gmail.com>
@srenatus
Copy link
Contributor

#4128

@srenatus
Copy link
Contributor

📢 Just a heads-up, we’ll include some OpenTelemetry support in the next release — if you have any specific needs or desires related to distributed tracing (what’s in a span, trace errors or not, create resources? events?!), please don’t hesitate to comment on the issues (e.g. this one or #4128), PRs (e.g. #4136), etc. 📶

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants