Add new metric to show duration all pipelineruns have taken #1764
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1764 +/- ##
==========================================
+ Coverage 65.11% 65.16% +0.04%
==========================================
Files 174 174
Lines 13185 13246 +61
==========================================
+ Hits 8586 8632 +46
- Misses 4030 4041 +11
- Partials 569 573 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
/test |
1 similar comment
|
/test |
pkg/reconciler/emit_metrics_test.go
Outdated
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "knative.dev/pkg/apis" | ||
| duckv1 "knative.dev/pkg/apis/duck/v1" | ||
| ) | ||
|
|
||
| func TestEmitMetrics(t *testing.T) { |
There was a problem hiding this comment.
you are not adding any new tests here, just fixing the current one?
There was a problem hiding this comment.
yeah, adjusted existing one to accommodate new metric.
There was a problem hiding this comment.
can't figure out how, because there is only one func emit_metrics, can you please guide how?
There was a problem hiding this comment.
I do not know how, without having to spend time on the code, what about a e2e test then?
There was a problem hiding this comment.
something like done in pipeline metric tests?
There was a problem hiding this comment.
Thank you @zakisk
Few comments and also can you add e2e to test both the metrics
zakisk
force-pushed
the
SRVKP-6226
branch
2 times, most recently
from
5a0297f
to
eb56bc4
Compare
added new metric to show sum of durations all pipelineruns have taken in seconds. added docs and test accordingly https://issues.redhat.com/browse/SRVKP-6226 Signed-off-by: Zaki Shaikh <[email protected]>
|
/test |
@savitaashture about adding E2E tests, I will need to do some extra work, like adding an ingress to access metrics endpoint in tests, metrics endpoint doesn't return response in JSON so need to figure out a consistent way to find metrics values in response so that it will be reusable across all PAC metrics E2E tests, so I am thinking doing that in a separate SRVKP. |
|
/lgtm Thank you |
added new metric to show sum of durations all pipelineruns have taken in seconds. added docs and adjusted test accordingly
https://issues.redhat.com/browse/SRVKP-6226
Changes
Submitter Checklist
📝 Please ensure your commit message is clear and informative. For guidance on crafting effective commit messages, refer to the How to write a git commit message guide. We prefer the commit message to be included in the PR body itself rather than a link to an external website (ie: Jira ticket).
♽ Before submitting a PR, run make test lint to avoid unnecessary CI processing. For an even more efficient workflow, consider installing pre-commit and running pre-commit install in the root of this repository.
✨ We use linters to maintain clean and consistent code. Please ensure you've run make lint before submitting a PR. Some linters offer a --fix mode, which can be executed with the command make fix-linters (ensure markdownlint and golangci-lint tools are installed first).
📖 If you're introducing a user-facing feature or changing existing behavior, please ensure it's properly documented.
🧪 While 100% coverage isn't a requirement, we encourage unit tests for any code changes where possible.
🎁 If feasible, please check if an end-to-end test can be added. See README for more details.
🔎 If there's any flakiness in the CI tests, don't necessarily ignore it. It's better to address the issue before merging, or provide a valid reason to bypass it if fixing isn't possible (e.g., token rate limitations).