8000 instrumentation: add run.LogCommands, run.TraceCommands by bobheadxi · Pull Request #42 · sourcegraph/run · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@bobheadxi
Copy link
Member
@bobheadxi bobheadxi commented Oct 12, 2022

Proposes two ways to instrument an application that executes a lot of commands you might want to debug:

  1. run.TraceCommands toggles OpenTelemetry tracing of commands executed within a context
  2. run.LogCommands toggles logging of executed commands within a context

See instrumentation_test.go for sample usages.

In https://github.com/sourcegraph/controller, tracing could become used as audit log type things, while logging could be useful for debugging in general.

Open to ideas here :)

@bobheadxi bobheadxi requested a review from a team October 12, 2022 23:31
@bobheadxi bobheadxi requested a review from michaellzc as a code owner October 12, 2022 23:31
@codecov-commenter
Copy link
codecov-commenter commented Oct 12, 2022

Codecov Report

Merging #42 (4355724) into main (e72b3f8) will increase coverage by 3.82%.
The diff coverage is 91.95%.

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   73.95%   77.77%   +3.82%     
==========================================
  Files          10       11       +1     
  Lines         311      378      +67     
==========================================
+ Hits          230      294      +64     
- Misses         64       68       +4     
+ Partials       17       16       -1     
Impacted Files Coverage Δ
output.go 91.41% <88.88%> (+0.97%) ⬆️
command.go 60.00% <100.00%> (ø)
instrumentation.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bobheadxi bobheadxi force-pushed the instrumentation branch 3 times, most recently from 70e7b4e to 4707e24 Compare October 12, 2022 23:49
Copy link
@daxmc99 daxmc99 left a comment

Choose a reason for hiding this comment

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

We are going to use this shortly in our internal CLI!

Copy link
Member
@michaellzc michaellzc left a comment

Choose a reason for hiding this comment

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

good stuff

@bobheadxi bobheadxi merged commit 4ca28ed into main Oct 14, 2022
@bobheadxi bobheadxi deleted the instrumentation branch October 14, 2022 19:08
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

0