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

otelhttptrace: handle missing getconn hook without panic #5965

Merged

Conversation

krantideep95
Copy link
Contributor

Fork of #5187 updated with main branch and tests, this PR adds nil dereference check for clientTracer.root in end() when span events are used instead of sub spans

@krantideep95 krantideep95 requested a review from dmathieu as a code owner July 31, 2024 23:10
@krantideep95 krantideep95 requested a review from a team July 31, 2024 23:10
Copy link

linux-foundation-easycla bot commented Jul 31, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@krantideep95 krantideep95 changed the title Httptrace end panic with tests otelhttptrace: handle missing getconn hook without panic Jul 31, 2024
@krantideep95 krantideep95 force-pushed the httptrace-end-panic-with-tests branch 2 times, most recently from 657953b to bb96c05 Compare August 1, 2024 09:27
Copy link

codecov bot commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.7%. Comparing base (7d7d4c3) to head (bf05062).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #5965   +/-   ##
=====================================
  Coverage   65.7%   65.7%           
=====================================
  Files        204     204           
  Lines      13083   13085    +2     
=====================================
+ Hits        8605    8609    +4     
+ Misses      4215    4214    -1     
+ Partials     263     262    -1     
Files with missing lines Coverage Δ
...on/net/http/httptrace/otelhttptrace/clienttrace.go 80.0% <100.0%> (+0.1%) ⬆️

... and 1 file with indirect coverage changes

@krantideep95 krantideep95 force-pushed the httptrace-end-panic-with-tests branch 2 times, most recently from ad46607 to 1a053a4 Compare August 5, 2024 16:44
@krantideep95
Copy link
Contributor Author

Hey @dmathieu, sorry for the tag. Would you mind reviewing this Pull Request? Thanks!

tonistiigi and others added 3 commits September 10, 2024 23:32
We have many reports that end() gets called without the
span being defined in start() and causes a panic.

Signed-off-by: Tonis Tiigi <[email protected]>
@krantideep95 krantideep95 force-pushed the httptrace-end-panic-with-tests branch 3 times, most recently from 1ee5129 to 7ecfc42 Compare September 10, 2024 22:40
@krantideep95 krantideep95 force-pushed the httptrace-end-panic-with-tests branch from 7ecfc42 to 6e6185e Compare September 10, 2024 22:41
@dmathieu dmathieu merged commit 042f99b into open-telemetry:main Sep 12, 2024
27 checks passed
@MrAlias MrAlias added this to the v1.31.0 milestone Oct 10, 2024
MrAlias added a commit that referenced this pull request Oct 14, 2024
### Added

- The `Severitier` and `SeverityVar` types are added to
`go.opentelemetry.io/contrib/processors/minsev` allowing dynamic
configuration of the severity used by the `LogProcessor`. (#6116)
- Move examples from `go.opentelemetry.io/otel` to this repository under
`examples` directory. (#6158)
- Support yaml/json struct tags for generated code in
`go.opentelemetry.io/contrib/config`. (#5433)
- Add support for parsing YAML configuration via `ParseYAML` in
`go.opentelemetry.io/contrib/config`. (#5433)
- Add support for temporality preference configuration in
`go.opentelemetry.io/contrib/config`. (#5860)

### Changed

- The function signature of `NewLogProcessor` in
`go.opentelemetry.io/contrib/processors/minsev` has changed to accept
the added `Severitier` interface instead of a `log.Severity`. (#6116)
- Updated `go.opentelemetry.io/contrib/config` to use the
[v0.3.0](https://github.com/open-telemetry/opentelemetry-configuration/releases/tag/v0.3.0)
release of schema which includes backwards incompatible changes. (#6126)
- `NewSDK` in `go.opentelemetry.io/contrib/config` now returns a no-op
SDK if `disabled` is set to `true`. (#6185)
- The deprecated
`go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho`
package has found a Code Owner. The package is no longer deprecated.
(#6207)

### Fixed

- Possible nil dereference panic in
`go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace`.
(#5965)
- `logrus.Level` transformed to appropriate `log.Severity` in
`go.opentelemetry.io/contrib/bridges/otellogrus`. (#6191)

### Removed

- The `Minimum` field of the `LogProcessor` in
`go.opentelemetry.io/contrib/processors/minsev` is removed.
  Use `NewLogProcessor` to configure this setting. (#6116)
- The deprecated
`go.opentelemetry.io/contrib/instrumentation/gopkg.in/macaron.v1/otelmacaron`
package is removed. (#6186)
- The deprecated `go.opentelemetry.io/contrib/samplers/aws/xray` package
is removed. (#6187)

---------

Co-authored-by: David Ashpole <[email protected]>
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.

5 participants