Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

DEVPROD-1661 add disk metrics to honeycomb metrics link #2183

Merged
merged 3 commits into from
Dec 14, 2023
Merged

Conversation

ybrill
Copy link
Contributor

@ybrill ybrill commented Dec 11, 2023

DEVPROD-1661

Description

This ought to be the last of the PRs for this feature. This modifies the Honeycomb system metrics link to include disk stats for the disk names that were recorded when a task finished running.

Screenshots

N/A

Testing

This query got generated for a task with a distro that had two disk devices configured.

Evergreen PR

evergreen-ci/evergreen#7308

@ybrill ybrill requested a review from a team December 11, 2023 20:25
Copy link

cypress bot commented Dec 11, 2023

1 failed test on run #14643 ↗︎

1 542 10 0 Flakiness 0

Details:

suggestions
Project: Spruce Commit: 4383ae7516
Status: Failed Duration: 25:07 💡
Started: Dec 13, 2023 11:04 PM Ended: Dec 13, 2023 11:29 PM
Failed  cypress/integration/preferences/public_key_management.ts • 1 failed test

View Output Video

Test Artifacts
Public Key Management Page > Edit Key Modal > After submitting, the key name and key value are updated Screenshots Video

Review all test suite changes for PR #2183 ↗︎

@ybrill
Copy link
Contributor Author

ybrill commented Dec 13, 2023

evergreen retry

Comment on lines 127 to 140
].concat(
diskDevices
.map((device) => [
{ op: "RATE_AVG", column: `system.disk.io.${device}.read` },
{ op: "RATE_AVG", column: `system.disk.io.${device}.write` },
{ op: "RATE_AVG", column: `system.disk.operations.${device}.read` },
{
op: "RATE_AVG",
column: `system.disk.operations.${device}.write`,
},
{ op: "RATE_AVG", column: `system.disk.io_time.${device}` },
])
.flat()
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Disclaimer that the lint might be wrong so I wouldn't suggest committing this suggestion via the button 😅

Suggested change
].concat(
diskDevices
.map((device) => [
{ op: "RATE_AVG", column: `system.disk.io.${device}.read` },
{ op: "RATE_AVG", column: `system.disk.io.${device}.write` },
{ op: "RATE_AVG", column: `system.disk.operations.${device}.read` },
{
op: "RATE_AVG",
column: `system.disk.operations.${device}.write`,
},
{ op: "RATE_AVG", column: `system.disk.io_time.${device}` },
])
.flat()
),
...diskDevices
.flatMap((device) => [
{ op: "RATE_AVG", column: `system.disk.io.${device}.read` },
{ op: "RATE_AVG", column: `system.disk.io.${device}.write` },
{ op: "RATE_AVG", column: `system.disk.operations.${device}.read` },
{
op: "RATE_AVG",
column: `system.disk.operations.${device}.write`,
},
{ op: "RATE_AVG", column: `system.disk.io_time.${device}` },
])
],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice! done

Comment on lines 83 to 84
describe("getTaskSystemMetricsUrlWithDisks", () => {
it("generates the correct url", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend just adding this test to the block above, since they test the same function! And renaming the block to getHoneycombSystemMetricsUrl (maybe getTaskSystemMetricsUrl was the old name?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ybrill ybrill merged commit a7bac9f into main Dec 14, 2023
3 of 6 checks passed
@ybrill ybrill deleted the DEVPROD-1661_link branch December 14, 2023 15:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants