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

otelzap: Add caller and stacktrace to attributes if present #6268

Merged
merged 14 commits into from
Oct 24, 2024

Conversation

suniastar
Copy link
Contributor

@suniastar suniastar commented Oct 21, 2024

  • Adds caller information to the log.Record if the logger was created with AddCaller().
  • Adds stack trace to the log.Record if the logger was created with AddStacktrace(level).

@suniastar suniastar requested review from pellared and a team as code owners October 21, 2024 11:17
Copy link

linux-foundation-easycla bot commented Oct 21, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@suniastar suniastar changed the title feat(otelzap): Added Caller to Record Attributes feat(otelzap): added caller to record attributes Oct 21, 2024
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a changelog entry.

bridges/otelzap/core.go Outdated Show resolved Hide resolved
@suniastar suniastar changed the title feat(otelzap): added caller to record attributes feat(otelzap): added caller and stack trace to record attributes Oct 21, 2024
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. Can you add a unit test to cover the new functionality?

CHANGELOG.md Outdated Show resolved Hide resolved
@suniastar
Copy link
Contributor Author

I have added tests for a zap.Logger created with and without the zap.AddCaller and zap.AddStacktrace(level) options.

@dmathieu
Copy link
Member

cc @khushijain21 as code owner

Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.6%. Comparing base (64d6d83) to head (bf0da84).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6268   +/-   ##
=====================================
  Coverage   66.6%   66.6%           
=====================================
  Files        192     192           
  Lines      15525   15535   +10     
=====================================
+ Hits       10351   10361   +10     
  Misses      4883    4883           
  Partials     291     291           
Files with missing lines Coverage Δ
bridges/otelzap/core.go 97.7% <100.0%> (+0.1%) ⬆️

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simplify the tests.

bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
@pellared pellared changed the title feat(otelzap): added caller and stack trace to record attributes otelzap: Add caller and stacktrace attributes if present Oct 22, 2024
@pellared pellared changed the title otelzap: Add caller and stacktrace attributes if present otelzap: Add caller and stacktrace to attributes if present Oct 22, 2024
@pellared
Copy link
Member

@suniastar thank you for your contribution. I plan to merge it tomorrow unless nobody asks for more changes.

@suniastar
Copy link
Contributor Author

Sound goods 👍🏻 Big thanks for the help everyone!

@pellared
Copy link
Member

pellared commented Oct 22, 2024

@suniastar, can you check the build result? Sorry, but currently I have no time to look at it.

pellared

This comment was marked as duplicate.

@suniastar
Copy link
Contributor Author

It fails on Check clean repository within the lint workflow but I am afraid I cannot tell what is meant by that.

@pellared
Copy link
Member

Run go mod tidy in otelzap directory .

@suniastar
Copy link
Contributor Author

Ah ok thank you. Now the error makes sense 😅

@dmathieu dmathieu merged commit 45bfcef into open-telemetry:main Oct 24, 2024
25 checks passed
@pellared pellared added this to the v1.32.0 milestone Nov 8, 2024
pellared added a commit that referenced this pull request Nov 8, 2024
### Added

- Add the `WithSource` option to the
`go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the
`code.*` attributes in the log record that includes the source location
where the record was emitted. (#6253)
- Add `ContextWithStartTime` and `StartTimeFromContext` to
`go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`, which
allows setting the start time using go context. (#6137)
- Set the `code.*` attributes in
`go.opentelemetry.io/contrib/bridges/otelzap` if the `zap.Logger` was
created with the `AddCaller` or `AddStacktrace` option. (#6268)
- Add a `LogProcessor` to
`go.opentelemetry.io/contrib/processors/baggagecopy` to copy baggage
members to log records. (#6277)
  - Use `baggagecopy.NewLogProcessor` when configuring a Log Provider.
- `NewLogProcessor` accepts a `Filter` function type that selects which
baggage members are added to the log record.

### Changed 

- Transform raw (`slog.KindAny`) attribute values to matching
`log.Value` types.
For example, `[]string{"foo", "bar"}` attribute value is now transformed
to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))`
instead of `log.String("[foo bar"])`. (#6254)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.17.0` to
`go.opentelemetry.io/otel/semconv/v1.21.0` in
`go.opentelemetry.io/contrib/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo`.
(#6272)
- Resource doesn't merge with defaults if a valid resource is configured
in `go.opentelemetry.io/contrib/config`. (#6289)

### Fixed

- Transform nil attribute values to `log.Value` zero value instead of
panicking in `go.opentelemetry.io/contrib/bridges/otellogrus`. (#6237)
- Transform nil attribute values to `log.Value` zero value instead of
panicking in `go.opentelemetry.io/contrib/bridges/otelzap`. (#6237)
- Transform nil attribute values to `log.Value` zero value instead of
`log.StringValue("<nil>")` in
`go.opentelemetry.io/contrib/bridges/otelslog`. (#6246)
- Fix `NewClientHandler` so that `rpc.client.request.*` metrics measure
requests instead of responses and `rpc.client.responses.*` metrics
measure responses instead of requests in
`go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`.
(#6250)
- Fix issue in `go.opentelemetry.io/contrib/config` causing
`otelprom.WithResourceAsConstantLabels` configuration to not be
respected. (#6260)
- `otel.Handle` is no longer called on a successful shutdown of the
Prometheus exporter in `go.opentelemetry.io/contrib/config`. (#6299)
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.

4 participants