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

k6 Insights (2/2): Integrate request metadata output to cloud output v1 #3202

Merged
merged 5 commits into from
Jul 19, 2023

Conversation

Blinkuu
Copy link
Contributor

@Blinkuu Blinkuu commented Jul 17, 2023

What?

This PR incorporates the request metadata output into cloud output v1.

Why?

Due to cloud output v2 being at risk for v0.46.0, we decided to incorporate the new request metadata output into cloud output v1. This change is done to mitigate the risk of failing to deliver the new insights features.

Related PR(s)/Issue(s)

This PR is part of a PR chain. Previous PR - #3201.

Previous PRs:

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2023

Codecov Report

Merging #3202 (a5320f9) into master (1278f6e) will increase coverage by 0.00%.
The diff coverage is 57.00%.

❗ Current head a5320f9 differs from pull request most recent head 2a2f95c. Consider uploading reports for the commit 2a2f95c to get more accurate results

@@           Coverage Diff           @@
##           master    #3202   +/-   ##
=======================================
  Coverage   72.69%   72.70%           
=======================================
  Files         254      259    +5     
  Lines       19802    19864   +62     
=======================================
+ Hits        14396    14442   +46     
- Misses       4497     4519   +22     
+ Partials      909      903    -6     
Flag Coverage Δ
ubuntu 72.63% <57.00%> (?)
windows 72.55% <57.00%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
metrics/engine/ingester.go 91.48% <ø> (ø)
output/cloud/expv2/collect.go 92.59% <ø> (+0.51%) ⬆️
output/cloud/expv2/flush.go 91.04% <ø> (-0.85%) ⬇️
output/cloud/v1/output.go 57.45% <0.00%> (-10.36%) ⬇️
output/cloud/expv2/output.go 72.83% <7.69%> (+7.09%) ⬆️
cloudapi/insights/client.go 62.20% <12.50%> (-17.18%) ⬇️
cmd/run.go 74.27% <86.66%> (+0.11%) ⬆️
output/cloud/insights/collect.go 91.48% <91.48%> (ø)
metrics/sink.go 97.40% <96.29%> (+1.51%) ⬆️
cloudapi/config.go 90.90% <100.00%> (ø)
... and 6 more

... and 11 files with indirect coverage changes

@olegbespalov olegbespalov changed the base branch from master to refactor_request_metadata_output_to_separate_package July 17, 2023 13:12
@olegbespalov olegbespalov requested review from oleiade and removed request for mstoykov and imiric July 17, 2023 16:44
Copy link
Contributor

@olegbespalov olegbespalov left a comment

Choose a reason for hiding this comment

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

Let's keep config under a single source of truth.

And question about the runFlushRequestMetadatas

output/cloud/v1/output.go Outdated Show resolved Hide resolved
output/cloud/v1/output.go Show resolved Hide resolved
output/cloud/v1/output.go Outdated Show resolved Hide resolved
@olegbespalov olegbespalov added this to the v0.46.0 milestone Jul 18, 2023
olegbespalov
olegbespalov previously approved these changes Jul 19, 2023
oleiade
oleiade previously approved these changes Jul 19, 2023
Base automatically changed from refactor_request_metadata_output_to_separate_package to master July 19, 2023 10:05
@oleiade oleiade dismissed stale reviews from olegbespalov and themself July 19, 2023 10:05

The base branch was changed.

@Blinkuu Blinkuu force-pushed the integrate_insights_to_v1_output branch from 0844bcb to 2a2f95c Compare July 19, 2023 10:07
@oleiade oleiade self-requested a review July 19, 2023 12:12
@olegbespalov olegbespalov merged commit 703970e into master Jul 19, 2023
@olegbespalov olegbespalov deleted the integrate_insights_to_v1_output branch July 19, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants