-
Notifications
You must be signed in to change notification settings - Fork 118
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 traceid to decision logs #462
Add traceid to decision logs #462
Conversation
added traceid and spanid to decision log. test for checking traceid in decision log is not passing
added traceid and spanid to decision log. test for checking traceid in decision log is not passing
3a90821
to
fb90113
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LionOnTheChase the changes look good. Some comments inline. Thanks!
var found bool | ||
|
||
for _, entry = range consoleLogger.Entries() { | ||
t.Log(entry.Message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: We probably don't need to log the entry.
@@ -88,7 +92,7 @@ func TestMain(m *testing.M) { | |||
resp.body.count == 1 | |||
}` | |||
module := fmt.Sprintf(moduleFmt, ts.URL) | |||
pluginsManager, err := e2e.TestAuthzServerWithWithOpts(module, "envoy/authz/allow", ":9191", plugins.WithTracerProvider(tracerProvider)) | |||
pluginsManager, err := e2e.TestAuthzServerWithWithOpts(module, "envoy/authz/allow", ":9191", plugins.WithTracerProvider(tracerProvider), plugins.ConsoleLogger(consoleLogger)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you intend to use the console logger somewhere in this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashutosh-narkar , yes.
I'm using it to capture the log entries, identify a decision log in them, and then assert for the presence of trace_id and span_id in the decision log.
see line 155 to 159
|
||
//services := []string{"s1", "s3"} | ||
|
||
m.Register("test_plugin", &testPlugin{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please help me understand why we're registering this and how we're using it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed to enable decision_logs .
Looking at
opa-envoy-plugin/envoyauth/evaluation_test.go
Lines 212 to 220 in 064d640
m.Register("test_plugin", &testPlugin{}) | |
config, err := logs.ParseConfig([]byte(`{"plugin": "test_plugin"}`), nil, []string{"test_plugin"}) | |
if err != nil { | |
return nil, err | |
} | |
plugin := logs.New(config, m) | |
m.Register(logs.Name, plugin) | |
Please suggest if there is any better way.
@LionOnTheChase can you please sign your commit. Thanks! |
Signed-off-by: talip <Praneeth.Talishetti@in.pega.com>
Signed-off-by: talip <Praneeth.Talishetti@in.pega.com>
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2 to 3. - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@v2...v3) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.58.0 to 1.58.1. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.58.0...v1.58.1) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.58.1 to 1.58.2. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.58.1...v1.58.2) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Bumps [golang.org/x/tools](https://github.com/golang/tools) from 0.13.0 to 0.14.0. - [Release notes](https://github.com/golang/tools/releases) - [Commits](golang/tools@v0.13.0...v0.14.0) --- updated-dependencies: - dependency-name: golang.org/x/tools dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: talip <Praneeth.Talishetti@in.pega.com>
8fe8254
to
a0badb7
Compare
Closing in favor of #470. |
open-policy-agent/opa#5230
thank you for your patience @ashutosh-narkar . kindly review