Skip to content

feat: enable skaffold render to track telemetry#9020

Merged
renzodavid9 merged 5 commits intoGoogleContainerTools:mainfrom
renzodavid9:tracking-render-command
Aug 17, 2023
Merged

feat: enable skaffold render to track telemetry#9020
renzodavid9 merged 5 commits intoGoogleContainerTools:mainfrom
renzodavid9:tracking-render-command

Conversation

@renzodavid9
Copy link
Copy Markdown
Contributor

@renzodavid9 renzodavid9 commented Aug 14, 2023

Related: #9013

Description
This PR configures the skaffold render command to start tracking telemetry data when it is used. Currently the logic for emitting metrics is related to the logic for displaying the Skaffold survey URL, the opt-out message, and the version upgrade. These messages were corrupting the output when used, e.g, like skaffold render > manifest.yaml, due to everything was printed using stdout. Now we are changing from stdout to stderr to print those messages.

@renzodavid9 renzodavid9 force-pushed the tracking-render-command branch from b0fc468 to 337f97c Compare August 15, 2023 14:06
@renzodavid9 renzodavid9 changed the title Tracking render command feat: enable skaffold render to track telemetry Aug 15, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 15, 2023

Codecov Report

Merging #9020 (e03607d) into main (290280e) will decrease coverage by 6.86%.
Report is 1004 commits behind head on main.
The diff coverage is 49.90%.

@@            Coverage Diff             @@
##             main    #9020      +/-   ##
==========================================
- Coverage   70.48%   63.62%   -6.86%     
==========================================
  Files         515      624     +109     
  Lines       23150    31934    +8784     
==========================================
+ Hits        16317    20318    +4001     
- Misses       5776    10084    +4308     
- Partials     1057     1532     +475     
Files Changed Coverage Δ
cmd/skaffold/app/cmd/completion.go 13.04% <0.00%> (-1.25%) ⬇️
cmd/skaffold/app/cmd/config/list.go 65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go 88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go 54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go 100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 40.90% <0.00%> (-12.94%) ⬇️
cmd/skaffold/app/cmd/generate_pipeline.go 60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go 65.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_profiles.go 66.66% <ø> (ø)
... and 40 more

... and 419 files with indirect coverage changes

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

@renzodavid9 renzodavid9 force-pushed the tracking-render-command branch from a30634b to 43278b8 Compare August 15, 2023 17:59
@pull-request-size pull-request-size bot added size/M and removed size/S labels Aug 15, 2023
@pull-request-size pull-request-size bot added size/L and removed size/M labels Aug 17, 2023
}

func DisplayMetricsPrompt(configFile string, out io.Writer) error {
if isStdOut(out) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was checking the idea behind this condition, it was checking if the given io.Writer was a stdout, but from my understanding in the code, we were always sending an stdout, so it was always true, and I didn't see other place where we were using this function or sending other type of io.Writer, so I removed the condition + function completely 😅 . Let me know if you see a case I'm not taking into account where this condition can be false.

@renzodavid9 renzodavid9 marked this pull request as ready for review August 17, 2023 15:39
@renzodavid9 renzodavid9 merged commit 2ba7d48 into GoogleContainerTools:main Aug 17, 2023
renzodavid9 added a commit that referenced this pull request Sep 7, 2023
* feat: making survey, update, and tracking messages to print to stderr instead of stdout

* feat: enable `skaffold render` to track telemetry

* feat: remove tests checking if a stadout is set, now we are using stderr

* feat: make stderr used to print survey, update, and metricts promp, coloreable to keep same behaviour as stdin

* feat: removing IsStdout function and related tests
renzodavid9 added a commit that referenced this pull request Sep 8, 2023
* feat: configure verify and exec commands to emit metrics (#9013)

* feat: configure render, verify, and exec commands to emit metrics

* fix: remove render command from tracking due to corrupted output data

* feat: enable skaffold render to track telemetry (#9020)

* feat: making survey, update, and tracking messages to print to stderr instead of stdout

* feat: enable `skaffold render` to track telemetry

* feat: remove tests checking if a stadout is set, now we are using stderr

* feat: make stderr used to print survey, update, and metricts promp, coloreable to keep same behaviour as stdin

* feat: removing IsStdout function and related tests

* fix: change baseRef to point to v2.6 release
Sign up for free to 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.

2 participants