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 traceid to decision logs #462

Closed

Conversation

be-a-bee
Copy link
Contributor

open-policy-agent/opa#5230

thank you for your patience @ashutosh-narkar . kindly review

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
Copy link
Member

@ashutosh-narkar ashutosh-narkar left a 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)
Copy link
Member

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))
Copy link
Member

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?

Copy link
Contributor Author

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{})
Copy link
Member

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?

Copy link
Contributor Author

@be-a-bee be-a-bee Oct 19, 2023

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

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)
, I figured that this is one way we could do it.

Please suggest if there is any better way.

@ashutosh-narkar
Copy link
Member

@LionOnTheChase can you please sign your commit. Thanks!

be-a-bee and others added 9 commits October 19, 2023 14:16
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>
@be-a-bee be-a-bee force-pushed the add-traceid-to-decision-logs branch from 8fe8254 to a0badb7 Compare October 19, 2023 08:48
@be-a-bee be-a-bee mentioned this pull request Oct 19, 2023
@ashutosh-narkar
Copy link
Member

Closing in favor of #470.

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 this pull request may close these issues.

2 participants