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

Add Crossplane Metrics Documentation #802

Merged
merged 10 commits into from
Sep 10, 2024

Conversation

Shwethamuralikrishnaa
Copy link
Contributor

This PR introduces a new documentation page dedicated to the metrics collected from Crossplane. The page is designed to help users understand the metric and how it can be used to monitor and troubleshoot Crossplane operations

Copy link

netlify bot commented Aug 23, 2024

Deploy Preview for crossplane ready!

Name Link
🔨 Latest commit 9c4d1fd
🔍 Latest deploy log https://app.netlify.com/sites/crossplane/deploys/66e06d850610c100081b6c44
😎 Deploy Preview https://deploy-preview-802--crossplane.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 83 (🔴 down 1 from production)
Accessibility: 90 (🔴 down 2 from production)
Best Practices: 83 (no change from production)
SEO: 93 (no change from production)
PWA: 70 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@Shwethamuralikrishnaa Shwethamuralikrishnaa force-pushed the add-metrics-readme branch 3 times, most recently from 35f9e7e to 9a60cd0 Compare August 23, 2024 14:13
@pierluigilenoci
Copy link
Contributor

@turkenh @plumbis, could you please take a look?

@pierluigilenoci
Copy link
Contributor

@jbw976, could you please take a look?

@AdamRussak
Copy link

from first look i found a syntax error in your MD: the table formating header i think.
image

Signed-off-by: shwethamuralikrishnaa [email protected]

Signed-off-by: shwethamuralikrishnaa <[email protected]>
Signed-off-by: Pierluigi Lenoci <[email protected]>
Signed-off-by: Pierluigi Lenoci <[email protected]>
@pierluigilenoci
Copy link
Contributor

@AdamRussak, those headers are the same across all pages.
For example,

{{<table "table table-sm">}}
We never touched this document.

Copy link
Collaborator

@jeanduplessis jeanduplessis left a comment

Choose a reason for hiding this comment

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

@Shwethamuralikrishnaa @pierluigilenoci Thanks for this valuable contribution. Could we please add (duplicate) the metrics page under the /content/v1.17 path as well please.

@jbw976 I'm wondering if "Concepts" is the best area for this content? Feels like a mixture of guides/API reference as well 🤔. That said I don't strong about this, its better to have this info in the docs than bikeshedding about where it should be.

@jeanduplessis
Copy link
Collaborator

@pierluigilenoci please have a look at signing off your commits so it can pass the DCO check
CleanShot 2024-09-09 at 11 06 00@2x

@pierluigilenoci
Copy link
Contributor

@jeanduplessis all commit signed, duplicated the content in v1.17
@AdamRussak, your suggestion has now been integrated into the PR.

Copy link
Collaborator

@jeanduplessis jeanduplessis left a comment

Choose a reason for hiding this comment

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

I'm happy with these as a good start that can be iterated on going forward. I'll wait for @jbw976 to confirm he's happy with the location of the content.

@jeanduplessis
Copy link
Collaborator

@pierluigilenoci removing of the table header has caused the build to fail
CleanShot 2024-09-09 at 11 43 25@2x

@AdamRussak
Copy link

Why?
I tested it in codespace, it is just showing Markdown syntax and is not converted to ui view.

@pierluigilenoci removing of the table header has caused the build to fail
CleanShot 2024-09-09 at 11 43 25@2x

Signed-off-by: Pierluigi Lenoci <[email protected]>
@pierluigilenoci
Copy link
Contributor

@AdamRussak solved. Could you please approve it again?

Signed-off-by: Pierluigi Lenoci <[email protected]>
| {{<hover label="crossplane_managed_resource_first_time_to_readiness_seconds_bucket" line="27">}}crossplane_managed_resource_first_time_to_readiness_seconds_bucket{{</hover>}} | The time it took for a managed resource to become ready first time after creation | |
| {{<hover label="crossplane_managed_resource_first_time_to_reconcile_seconds_bucket" line="28">}}crossplane_managed_resource_first_time_to_reconcile_seconds_bucket{{</hover>}} | The time it took to detect a managed resource by the controller | |
| {{<hover label="upjet_resource_ttr_bucket" line="29">}}upjet_resource_ttr_bucket{{</hover>}} | Measures in seconds the `time-to-readiness` `(TTR)` for managed resources | |
{{< /table >}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please close this?

| {{<hover label="crossplane_managed_resource_first_time_to_readiness_seconds_bucket" line="27">}}crossplane_managed_resource_first_time_to_readiness_seconds_bucket{{</hover>}} | The time it took for a managed resource to become ready first time after creation | |
| {{<hover label="crossplane_managed_resource_first_time_to_reconcile_seconds_bucket" line="28">}}crossplane_managed_resource_first_time_to_reconcile_seconds_bucket{{</hover>}} | The time it took to detect a managed resource by the controller | |
| {{<hover label="upjet_resource_ttr_bucket" line="29">}}upjet_resource_ttr_bucket{{</hover>}} | Measures in seconds the `time-to-readiness` `(TTR)` for managed resources | |
{{< /table >}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please close this?

@pierluigilenoci
Copy link
Contributor

@jbw976, please give us feedback.

@negz
Copy link
Member

negz commented Sep 10, 2024

@jbw976 I'm wondering if "Concepts" is the best area for this content?

FWIW that was my first thought too. I don't think this is a Crossplane concept. Not sure where the best fit for it is though.

Comment on lines 7 to 9
This page offers explanations of various metrics gathered from Crossplane, which are essential for effective monitoring and alerting in your Crossplane environment.
Understanding these metrics help you maintain the health and performance of your resources, ensuring to identify and address any issues.
Please note that this document focuses on Crossplane specific metrics and doesn't cover standard Go metrics.
Copy link
Member

Choose a reason for hiding this comment

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

It'd be useful to mention:

  • That these are prometheus style metrics
  • Where they're exposed (i.e. the port and URL)
  • How to enable them (iirc they're not on by default)

I don't think it's worth explaining what Prometheus style metrics are or how they work, but we should link to the Prometheus documentation for those who are unfamiliar.

Copy link
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

Really cool to see you take the initiative to document this useful information
@Shwethamuralikrishnaa and @pierluigilenoci. Thank you for taking the time to do so!

I know Vale is a bit of a pain, but hopefully that pain is reduced some with all the fixes we have cooking in #810. I will hold off on merging that one though so you don't have to deal with any merge conflicts 😇

Please note that this document focuses on Crossplane specific metrics and doesn't cover standard Go metrics.


| Metric Name | Description | Further Explanation |
Copy link
Member

@jbw976 jbw976 Sep 10, 2024

Choose a reason for hiding this comment

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

on the preview site, the table is a bit hard to read/parse because there are no visible rows: https://deploy-preview-802--crossplane.netlify.app/v1.17/concepts/metrics/

Take a look at the table for Customize the Crossplane Helm chart on
https://github.com/crossplane/docs/blob/master/content/master/software/install.md#customize-the-crossplane-helm-chart

It does some styling directives to make easily visible striped rows - see if that helps with this new table!

For example:

{{< table "table table-hover table-striped table-sm">}}

@pierluigilenoci
Copy link
Contributor

@negz @jeanduplessis, please tell me where to move it, or just be happy, and we'll move it later. 😜
@negz I'll add a note about Prometheus metrics. However, if someone wants to use Kubernetes and Crossplane nowadays but doesn't know Prometheus metrics, maybe it's better for them not to.
@jbw976 I'll try to integrate the annotations. I had already tried, but the pipeline was broken. Ref: #802 (comment)

Signed-off-by: Pierluigi Lenoci <[email protected]>
Signed-off-by: Pierluigi Lenoci <[email protected]>
Signed-off-by: Pierluigi Lenoci <[email protected]>
@pierluigilenoci
Copy link
Contributor

@negz integrated your suggestions and moved the page from Concept to Guides.
@jbw976 added the directives; let's see what happens once approved.

@jbw976
Copy link
Member

jbw976 commented Sep 10, 2024

Deployment preview succeeded and the table with the colored rows looks great @pierluigilenoci!!
https://deploy-preview-802--crossplane.netlify.app/v1.17/guides/metrics/

Don't worry about any of the Vale failures, I'll make sure it's still all clean and green in #810 🟢

Signed-off-by: Pierluigi Lenoci <[email protected]>
@pierluigilenoci
Copy link
Contributor

@jbw976, it's too late! 😄

@pierluigilenoci
Copy link
Contributor

@jbw976, what needs to be added to merge with PR if the page looks great and everything else is 🟢 ?

Copy link
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

Thank you very much for continuing to drive this PR and get this important topic documented! 🙇‍♂️

@jbw976 jbw976 merged commit 5dbfab8 into crossplane:master Sep 10, 2024
7 checks passed
@pierluigilenoci pierluigilenoci deleted the add-metrics-readme branch September 11, 2024 08:48
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.

6 participants