Skip to content

Plumb contexts through commands - #4700

Merged
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:wire_up_contexts
Dec 13, 2023
Merged

Plumb contexts through commands#4700
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:wire_up_contexts

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

This is to prepare for otel support.

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztahthaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Dec 6, 2023
@codecov-commenter

codecov-commenter commented Dec 6, 2023

Copy link
Copy Markdown

Codecov Report

Merging #4700 (5400a48) into master (9eb632d) will decrease coverage by 0.02%.
The diff coverage is 67.07%.

Additional details and impacted files
@@ Coverage Diff @@## master #4700 +/- ##
==========================================
- Coverage 59.68% 59.66% -0.02% 
==========================================
Files 287 287 Lines 24865 24760 -105 ==========================================
- Hits 14841 14774 -67 + Misses 9138 9100 -38 
Partials 886 886 

Comment threadcli/command/container/stats.go Outdated
Comment on lines +130 to +131
span := trace.SpanFromContext(ctx)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

DOH! Looks like some OTEL changes landed in the wrong commit; let me fix that;

13 7.030 ERROR: Vendor result differs. Please vendor your package with "make -f docker.Makefile vendor"

@thaJeztah
thaJeztah marked this pull request as ready for review December 6, 2023 15:36
@thaJeztahthaJeztah mentioned this pull request Dec 6, 2023
@thaJeztahthaJeztah added this to the 25.0.0 milestone Dec 6, 2023
@thaJeztah
thaJeztahforce-pushed the wire_up_contexts branch 3 times, most recently from 84ecac1 to 4074b49CompareDecember 12, 2023 15:11
Comment threadcli/command/cli.go Outdated
This is to prepare for otel support.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@sam-thibaultsam-thibault left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@laurazardlaurazard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment threadcli/command/cli.go
// baseCtx is the base context used for internal operations. In the future
// this may be replaced by explicitly passing a context to functions that
// need it.
baseCtx context.Context

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should keep an eye on this and see if we can factor it out later 👀

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@thaJeztah
thaJeztah merged commit ae2b58f into docker:masterDec 13, 2023
@thaJeztah
thaJeztah deleted the wire_up_contexts branch December 13, 2023 21:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@thaJeztah@codecov-commenter@vvoland@laurazard@sam-thibault@cpuguy83