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.

Sign up for GitHub

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

Jump to bottom

OpenTelemetry follow-ups #4128

Closed
3 tasks done
srenatus opened this issue Dec 13, 2021 · 2 comments · Fixed by #4146
Closed
3 tasks done

OpenTelemetry follow-ups #4128

srenatus opened this issue Dec 13, 2021 · 2 comments · Fixed by #4146
Assignees
@srenatus

Comments

@srenatus
Copy link
Contributor

srenatus commented Dec 13, 2021 •
edited
Loading

Mixed bag:

  • v0 GET and v1 POST handlers need to pass along their config to rego
  • Is there any interaction with decision IDs that we'd like to have? Should the decision IDs get passed along with the span attributes, or should the span ID become part of a decision log entry?
  • Documentation: The section on Distributed Tracing could become part of Monitoring instead of Configuration (I think?)
@srenatus srenatus mentioned this issue Dec 13, 2021
Closed
@srenatus
Copy link
Contributor Author

Bundles could also use it with its downloader.

@srenatus srenatus self-assigned this Dec 14, 2021
@srenatus
Copy link
Contributor Author

https://opentelemetry.io/docs/instrumentation/go/getting-started/#bonus-errors another thing that's possible ✨

@srenatus srenatus mentioned this issue Dec 14, 2021
Merged
srenatus added a commit that referenced this issue Dec 15, 2021
@srenatus
server: add tracing to remaining handlers, cleanup code (#4136)
d8bdd3b 
This deals with the first two bullets of #4128:

1. tracing for remaining handlers
2. decisions IDs are added to the server spans

I'm not sure if that's the convention, but I've put the decision ID into the server
spans: the client spans we get from http.send usage in policies will not carry
them, but they do refer to their parents, and they'll have the `opa.decision_id`
attribute.

Also includes some general cleanup:
* server/writer: use switch for ErrorAuto()
* server: replace http statuses with their constants

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@srenatus srenatus mentioned this issue Dec 15, 2021
Merged
srenatus added a commit that referenced this issue Dec 16, 2021
@srenatus
OpenTelemetry: docs, config tweaks, no empty decision ID attributes (#…
668708d 
…4146)

1. I think an extra section on OT is warranted, even if it's brief.

2. The config now expects "type" to be set to "grpc" to enable the current
   feature set. More options may follow in the future.

This is meant to future-proof the config. Future values might include
"http", where the address, TLS and sampling related settings would be
reused; and "global", where the setup would use whatever global trace
provider was configured with otel. The latter use case would be for
embedding OPA via the SDK.

3. Decision ID attributes are only added to spans if decision logging is enabled.

Fixes #4128.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srenatus srenatus

Labels
None yet
Projects
Open Policy Agent
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

OpenTelemetry: docs, config tweaks, no empty decision ID attributes srenatus/opa
1 participant
@srenatus