-
Notifications
You must be signed in to change notification settings - Fork 807
[e2e] Emit a duration-scoped grafana link for each test #3340
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
Conversation
0ea1618
to
ef2fb5e
Compare
5a02656
to
a11b047
Compare
a11b047
to
62d6253
Compare
62d6253
to
c3a4d13
Compare
a2eb7d0
to
501b151
Compare
Co-authored-by: Stephen Buttolph <stephen@avalabs.org> Signed-off-by: marun <maru.newby@avalabs.org>
// Ensure that this value takes into account the scrape_interval | ||
// defined in scripts/run_prometheus.sh. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this meant as a TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a TODO - this is a reminder to coordinate changes to this value with the value in the script.
FWIW this should be temporary - when I have the cycles I plan to integrate starting of the log and metrics collectors into the e2e framework and that will remove the need for duplicating configuration.
if env == nil || !EmitMetricsLink { | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is handling env == nil
necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This avoids a broken event handler in the case that there is no global env configured. This is the case for subnet-evm's load test suite for example, where the env is local-only:
https://github.com/ava-labs/subnet-evm/blob/master/tests/load/load_test.go#L60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future, please don't hesitate to wait until someone responds to merge. I would have been happy to add a comment here pre-merge.
Why this should be merged
When an e2e failure occurs in CI, it would be useful to start with a metrics link scoped to the duration of execution of the failing test.
How this works
e2e.EmitMetricsLink
tofalse
AfterEach
handler for the shared network UUID and the start/end times of the current specHow this was tested
CI