Skip to content

Feature implementation from commits 68da54d..f7d81db#1

Open
yashuatla wants to merge 15 commits into
feature-base-1from
feature-head-1
Open

Feature implementation from commits 68da54d..f7d81db#1
yashuatla wants to merge 15 commits into
feature-base-1from
feature-head-1

Conversation

@yashuatla
Copy link
Copy Markdown
Owner

This PR contains changes from a range of commits from the original repository.

Commit Range: 68da54d..f7d81db
Files Changed: 82 (42 programming files)
Programming Ratio: 51.2%

Commits included:

renovate Bot and others added 15 commits June 2, 2025 10:06
…try#7411)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…p/dup` with new/old metrics (open-telemetry#7356)

Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
…metry#7417)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…n-telemetry#7419)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…n-telemetry#7420)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
wantRecording: logtest.Recording{
logtest.Scope{Name: name}: []logtest.Record{
buildRecord(log.StringValue(""), time.Time{}, log.SeverityWarn, nil),
want: logtest.Recording{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Incorrect field name causing test failure.

The test case uses 'wantRecording' but the test function expects 'want', which would cause the test to fail.

Current Code (Diff):

- 			wantRecording: logtest.Recording{
+ 			want: logtest.Recording{
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
want: logtest.Recording{
want: logtest.Recording{

Comment on lines +318 to +319
s.requestBodySize.Inst().Record(ctx, md.RequestSize, opts["new"].MeasurementOption())
s.requestDuration.Inst().Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Nil pointer dereference risk.

Removing null checks for requestBodySize and requestDuration could cause runtime panics if HTTPClient is directly instantiated

Current Code (Diff):

+ 	if s.requestBodySize == nil || s.requestDuration == nil {
+ 		// This will happen if an HTTPClient{} is used instead of NewHTTPClient().
+ 		return
+ 	}
+ 
  	s.requestBodySize.Inst().Record(ctx, md.RequestSize, opts["new"].MeasurementOption())
  	s.requestDuration.Inst().Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption())
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
s.requestBodySize.Inst().Record(ctx, md.RequestSize, opts["new"].MeasurementOption())
s.requestDuration.Inst().Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption())
if s.requestBodySize == nil || s.requestDuration == nil {
// This will happen if an HTTPClient{} is used instead of NewHTTPClient().
return
}
s.requestBodySize.Inst().Record(ctx, md.RequestSize, opts["new"].MeasurementOption())
s.requestDuration.Inst().Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption())

Comment on lines +154 to +156
s.requestBodySizeHistogram.Inst().Record(ctx, md.RequestSize, *recordOpts...)
s.responseBodySizeHistogram.Inst().Record(ctx, md.ResponseSize, *recordOpts...)
s.requestDurationHistogram.Inst().Record(ctx, md.ElapsedTime/1000.0, o)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Potential Null Pointer Exception.

Removed null checks for histogram objects could cause application crashes if any histogram object is null when accessed

Current Code (Diff):

-	s.requestBodySizeHistogram.Inst().Record(ctx, md.RequestSize, *recordOpts...)
-	s.responseBodySizeHistogram.Inst().Record(ctx, md.ResponseSize, *recordOpts...)
-	s.requestDurationHistogram.Inst().Record(ctx, md.ElapsedTime/1000.0, o)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
s.requestBodySizeHistogram.Inst().Record(ctx, md.RequestSize, *recordOpts...)
s.responseBodySizeHistogram.Inst().Record(ctx, md.ResponseSize, *recordOpts...)
s.requestDurationHistogram.Inst().Record(ctx, md.ElapsedTime/1000.0, o)
if s.requestBodySizeHistogram != nil {
s.requestBodySizeHistogram.Inst().Record(ctx, md.RequestSize, *recordOpts...)
}
if s.responseBodySizeHistogram != nil {
s.responseBodySizeHistogram.Inst().Record(ctx, md.ResponseSize, *recordOpts...)
}
if s.requestDurationHistogram != nil {
s.requestDurationHistogram.Inst().Record(ctx, md.ElapsedTime/1000.0, o)
}

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