Skip to content

Conversation

@jkhelil
Copy link
Contributor

@jkhelil jkhelil commented Apr 24, 2024

A followup to #989 accidently, and not able to reopen it)

@gabemontero
Copy link
Collaborator

yeah I've never seen that being able to reopen a PR before @jkhelil .... the same thing shows up for me. No idea what is going on.

in any event, yes, the screen shot at #989 (comment) was along the lines of what I was talking about. Thanks.

Copy link
Collaborator

@gabemontero gabemontero left a comment

Choose a reason for hiding this comment

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

the queries look OK, but some of the titles etc. that you copied over from the results panel were not updated to say tekton chains @jkhelil

],
"title": "Tekton Chains CPU Fraction Use",
"type": "timeseries"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

any idea why this Tekton Chains CPU Fraction Use was repeated @jkhelil .... perhaps the title is wrong and it needs to changes to reflect what the new panel is ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate, removed now

"id": 79,
"panels": [
{
"description": "Watcher work queue depth",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"description": "Watcher work queue depth",
"description": "Tekton Chains work queue depth",

Copy link
Collaborator

Choose a reason for hiding this comment

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

You still have to this one to update @jkhelil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

"refId": "work queue depth"
}
],
"title": "Watcher Work Queue Depth",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"title": "Watcher Work Queue Depth",
"title": "Tekton Chains Work Queue Depth",

"refId": "reconcile latency"
}
],
"title": "Watcher Reconcile Latency",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"title": "Watcher Reconcile Latency",
"title": "Tekton Chains Reconcile Latency",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed hereis a final screenshot
image

Copy link
Collaborator

@gabemontero gabemontero left a comment

Choose a reason for hiding this comment

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

closer but a couple of more items @jkhelil

also, I suggest you try some of the queried from Konflux prod or stage if you have not already for practice

{
"editorMode": "code",
"exemplar": true,
"expr": "sum(watcher_workqueue_depth{container='tekton-chains-controller',app='tekton-chains-controller'})",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I validated this query works in prod-rh01 from the OCP metrics console.

In theory you should have permissions to do that as well @jkhelil

If you get a sec, try logging into the OCP console using the red hat SSO and try your query over at https://console-openshift-console.apps.stone-prd-rh01.pg1f.p1.openshiftapps.com/monitoring/query-browser?query0=

"id": 79,
"panels": [
{
"description": "Watcher work queue depth",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You still have to this one to update @jkhelil

{
"editorMode": "code",
"exemplar": true,
"expr": "histogram_quantile(0.99, sum(rate(watcher_reconcile_latency_bucket{job=\"tekton-chains\"}[30m])) by (le) ) / 1000",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had to convert \" to just " @jkhelil but histogram_quantile(0.99, sum(rate(watcher_reconcile_latency_bucket{job="tekton-chains"}[30m])) by (le) ) / 1000 also works from the OCP metrics console ... see if you can do it as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is just a way to escape doublequote inside the doublequote, it is used all over the json file

{
"editorMode": "builder",
"expr": "rate(watcher_go_gc_cpu_fraction{app=\"tekton-chains-controller\"}[5m])",
"expr": "rate(watcher_workqueue_longest_running_processor_seconds_count{app=\"tekton-chains-controller\"}[5m])",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that I'm necessarily opposed to this change, but this was not one we discussed @jkhelil ... explain to me why you went in this direction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there was no change,just inversed the order for the two metrics, it is now ok

{
"editorMode": "builder",
"expr": "rate(watcher_workqueue_longest_running_processor_seconds_count{app=\"tekton-chains-controller\"}[5m])",
"expr": "rate(watcher_go_gc_cpu_fraction{app=\"tekton-chains-controller\"}[5m])",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that I'm necessarily opposed to this change, but this was not one we discussed @jkhelil ... explain to me why you went in this direction

@gabemontero
Copy link
Collaborator

looks good diff wise @jkhelil
also checked out your PR and tried it locally
merging

@gabemontero gabemontero merged commit 387e135 into openshift-pipelines:main Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants