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

Allow clients to supply decision id #1451

Closed
omerlh opened this issue May 28, 2019 · 9 comments
Closed

Allow clients to supply decision id #1451

omerlh opened this issue May 28, 2019 · 9 comments

Comments

@omerlh
Copy link
Contributor

omerlh commented May 28, 2019

Expected Behavior

As a client, I want to specify the decision id when calling OPA instead of having OPA generate it. This will make it easy to return the id to the end user without exposing the fact that I'm using OPA - e.g. without adding header like x-decision-id.

Actual Behavior

Decision id is generated by OPA

Steps to Reproduce the Problem

  1. Enable decision logs
  2. Call /v1/data endpoint
  3. The decision id is generated automatically

Additional Info

@tsandall
Copy link
Member

Thanks for filing this @omerlh. We can certainly add the ability to supply the decision ID as a query argument. Callers should be required to supply globally unique values (e.g., we wouldn't want OPA emitting decision log events with duplicate IDs).

As far as implementation goes, I'd expect the V1 Data POST API to be the most logical place to start.

@omerlh
Copy link
Contributor Author

omerlh commented May 30, 2019

Yep, I started to work on a PR that add a query param to the v1 Data Post API. I hope my go env will start behaving so I can complete this PR :)

@patrick-east
Copy link
Contributor

Can you give some more details on the use-case for this? I can understand shielding the users from details about the implementation using OPA.. but why do they need the decision ID in the first place?

If it is for tracing or some lookup later can you not just pass the ID you want as part of the input and then add it into the result of your evaluation?

I'm wondering too if maybe we would be better off adding in support for OpenTracing contexts in the HTTP servers requests/responses. We could potentially use those to key off for the decision id's, and have a very structured/standardized way of specifying them for the API.

@omerlh
Copy link
Contributor Author

omerlh commented Jun 4, 2019

Yep, I was also wondering about OpenTracing. I guess it's pretty much the same as the work I already did, I'm just less familiar with the format. The idea was to allow the clients use an existing request id as the decision id - something that is already returned to the end user - for easy tracking of requests as you mentioned. I think that allowing clients to specify the decision is a good first step toward better traceability, but OpenTracing should be also supported. Maybe worth opening a different issue?

@patrick-east
Copy link
Contributor

Out of curiosity was there any reason to not just pass the external ID through the input and policy/query output? Kind of a silly example but something along the lines of: https://play.openpolicyagent.org/p/YR6SOqShE5

I guess maybe more to the point, does it need to be the decision ID?

@omerlh
Copy link
Contributor Author

omerlh commented Jun 4, 2019

I'm not OPA expert :) what will happen if I'll ask for play/hello? Or you're thinking that as user I should always ask for play and get back both hello and output?

@patrick-east
Copy link
Contributor

patrick-east commented Jun 4, 2019

I guess it depends on how OPA is being used. Maybe part of the issue is that I still don't really understand the exact scenario.

From the comments thus far it sounds like some client calls OPA for a decision, but it is part of some larger request workflow (hence the external request ID). If the input/output from OPA APIs are being handled by other services they can, by convention, just push the request ID along with the request/response. In the example that means querying for the whole thing (play) and parsing the output to be used however you need.

The part I'm not sure of is who/what is actually making these calls to OPA. At some level there must be code that handles parsing out decision ID's, right? Is it so different if it instead parses out the other request ID in the result?

@patrick-east
Copy link
Contributor

@omerlh I created #1469 to track the OpenTracing integration. After our discussion on slack earlier I think the conclusion was that passing the request id as input and referencing it in the decision logs would be sufficient for your use-case. Assuming that is going to be OK we can probably close the issue and the open PR.

I'll let you know when the stdout decision logger is available too, should be soon.

@omerlh
Copy link
Contributor Author

omerlh commented Jun 5, 2019

Yes, thank you for your time! For potential readers, a solution will be to send a custom decision id as part of the input:

{
  "input": 
  {
     "decision_id": "<>"
  }
}

The input is sent as with each decision log, so it should be possible to filter by this custom field without changing the code in OPA.

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

Successfully merging a pull request may close this issue.

3 participants