Skip to content

Commit

Permalink
Improve documentation and provide RequestContext type-safe accessors
Browse files Browse the repository at this point in the history
Explain that request ID is incremental and unique by OPA instance.

Define RequestContext key as an unexported const to avoid colision and
provide RequestContext type-safe accessors.
More info: https://pkg.go.dev/context#Context.Value

Fixes: #5006

Signed-off-by: Humberto Corrêa da Silva <humbertoc_silva@hotmail.com>
  • Loading branch information
humbertoc-silva committed Oct 14, 2022
1 parent 7273146 commit 02afff9
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 8 deletions.
2 changes: 1 addition & 1 deletion docs/content/management-decision-logs.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Decision log updates contain the following fields:
| `[_].erased` | `array[string]` | Set of JSON Pointers specifying fields in the event that were erased. |
| `[_].masked` | `array[string]` | Set of JSON Pointers specifying fields in the event that were masked. |
| `[_].nd_builtin_cache` | `object` | Key-value pairs of non-deterministic builtin names, paired with objects specifying the input/output mappings for each unique invocation of that builtin during policy evaluation. Intended for use in debugging and decision replay. Receivers will need to decode the JSON using Rego's JSON decoders. |
| `[_].req_id` | `number` | Unique request identifier for the request that started the policy query. The attribute value is the same as the value present in others logs (request, response, and print) and could be used to correlate them all. This attribute will be included just when OPA runtime is initialized in server mode and the log level is equal or greater than info. |
| `[_].req_id` | `number` | Incremental request identifier, and unique only to the OPA instance, for the request that started the policy query. The attribute value is the same as the value present in others logs (request, response, and print) and could be used to correlate them all. This attribute will be included just when OPA runtime is initialized in server mode and the log level is equal to or greater than info. |

If the decision log was successfully uploaded to the remote service, it should respond with an HTTP 2xx status. If the
service responds with a non-2xx status, OPA will requeue the last chunk containing decision log events and upload it
Expand Down
15 changes: 13 additions & 2 deletions logging/logging.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package logging

import (
"context"
"io"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -188,8 +189,7 @@ func (l *NoOpLogger) GetLevel() Level {

type requestContextKey string

// ReqCtxKey is the key used to access the request context.
const ReqCtxKey = requestContextKey("request-context-key")
const reqCtxKey = requestContextKey("request-context-key")

// RequestContext represents the request context used to store data
// related to the request that could be used on logs.
Expand All @@ -209,3 +209,14 @@ func (rctx RequestContext) Fields() logrus.Fields {
"req_path": rctx.ReqPath,
}
}

// NewContext returns a copy of parent with an associated RequestContext.
func NewContext(parent context.Context, val *RequestContext) context.Context {
return context.WithValue(parent, reqCtxKey, val)
}

// FromContext returns the RequestContext associated with ctx, if any.
func FromContext(ctx context.Context) (*RequestContext, bool) {
requestContext, ok := ctx.Value(reqCtxKey).(*RequestContext)
return requestContext, ok
}
5 changes: 2 additions & 3 deletions runtime/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package runtime

import (
"bytes"
"context"
"io"
"io/ioutil"
"net/http"
Expand All @@ -25,7 +24,7 @@ type loggingPrintHook struct {
func (h loggingPrintHook) Print(pctx print.Context, msg string) error {
// NOTE(tsandall): if the request context is not present then do not panic,
// just log the print message without the additional context.
rctx, _ := pctx.Context.Value(logging.ReqCtxKey).(logging.RequestContext)
rctx, _ := logging.FromContext(pctx.Context)
fields := rctx.Fields()
fields["line"] = pctx.Location.String()
h.logger.WithFields(fields).Info(msg)
Expand Down Expand Up @@ -64,7 +63,7 @@ func (h *LoggingHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
rctx.ClientAddr = r.RemoteAddr
rctx.ReqMethod = r.Method
rctx.ReqPath = r.URL.EscapedPath()
r = r.WithContext(context.WithValue(r.Context(), logging.ReqCtxKey, rctx))
r = r.WithContext(logging.NewContext(r.Context(), &rctx))

var err error
fields := rctx.Fields()
Expand Down
4 changes: 2 additions & 2 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2916,8 +2916,8 @@ func (l decisionLogger) Log(ctx context.Context, txn storage.Transaction, decisi
}

var reqID uint64
if rctx := ctx.Value(logging.ReqCtxKey); rctx != nil {
reqID = rctx.(logging.RequestContext).ReqID
if rctx, ok := logging.FromContext(ctx); ok {
reqID = rctx.ReqID
}

info := &Info{
Expand Down

0 comments on commit 02afff9

Please sign in to comment.