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.

Sign up for GitHub

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

Jump to bottom

Add metrics and alerts tutorial to the docs #6341

Merged
merged 1 commit into from
Mar 27, 2023
Merged

Add metrics and alerts tutorial to the docs #6341

merged 1 commit into from
Mar 27, 2023

Conversation

assafad
Copy link
Contributor

@assafad assafad commented Feb 28, 2023 •
edited

Description of the change:
Add a monitoring tutorial to the advanced-topics page. This tutorial explains how to expose the first operator's custom metrics and alerts, by demonstrating over the monitoring sample - memcached-operator.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@assafad
Copy link
Contributor Author

assafad commented Feb 28, 2023

/cc @sradco

@openshift-ci
Copy link

openshift-ci bot commented Feb 28, 2023

@assafad: GitHub didn't allow me to request PR reviews from the following users: sradco.

Note that only operator-framework members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @sradco

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@assafad assafad force-pushed the monitoring-docs branch 3 times, most recently from baeaa34 to d92454e Compare February 28, 2023 10:21
@assafad assafad force-pushed the monitoring-docs branch 2 times, most recently from 39fdf24 to c657fac Compare February 28, 2023 13:53
sradco
sradco reviewed Feb 28, 2023
View reviewed changes
website/content/en/docs/building-operators/golang/advanced-topics.md Outdated
- Create a new `monitoring` directory, which will contain the metrics and alerts implementation. It is recommended to create a dedicated `monitoring` directory, in order to keep the monitoring implementation separated from the core operator's logic. This will allow easier maintenance of both the core operator code and the monitoring code.
- Add the following command to the `dockerfile`:
```dockerfile
COPY monitoring/ monitoring/
Copy link
Contributor

@sradco sradco Feb 28, 2023 •
edited

Choose a reason for hiding this comment

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

I think this pre-requisite and the one before should be in a separate header like:

#### Add Monitoring to Your Operator  

For a quick start, Copy the example `/monitoring` sub folder from the Memcached operator[add link to the example operator] and update it to you needs.  
This sub folder will include all the monitoring related code and logic.
**Note:** We created a dedicated `/monitoring` directory, since it is recommended to keep the monitoring code and the core operator code separated.

@assafad assafad force-pushed the monitoring-docs branch 7 times, most recently from 6cff809 to 099d163 Compare March 6, 2023 15:36
@assafad assafad force-pushed the monitoring-docs branch 2 times, most recently from 2ee9199 to 64920d9 Compare March 7, 2023 08:48
@assafad
Copy link
Contributor Author

assafad commented Mar 7, 2023

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 7, 2023
everettraven
everettraven approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

Just a few nits, other than that looks good to me! Thanks @assafad for working on this!

website/content/en/docs/building-operators/golang/advanced-topics.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/golang/advanced-topics.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/golang/advanced-topics.md Outdated Show resolved Hide resolved
@assafad assafad force-pushed the monitoring-docs branch 3 times, most recently from d15160e to 29981a6 Compare March 8, 2023 17:57
@assafad
Copy link
Contributor Author

assafad commented Mar 15, 2023

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2023
assafad
assafad commented Mar 15, 2023
View reviewed changes
website/content/en/docs/building-operators/golang/advanced-topics.md Outdated


where we assume that the prometheus service is available in the `monitoring` namespace.
Now you can access Prometheus UI using [http://localhost:9090](http://localhost:9090).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@everettraven, @varshaprasad96 I think sanity fails because this URL is not accessible. How can we handle that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@everettraven @varshaprasad96 any idea?

Copy link
Member

@varshaprasad96 varshaprasad96 Mar 24, 2023 •
edited

Choose a reason for hiding this comment

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

As a temporary workaround, we need not add it as a link:

Suggested change
Now you can access Prometheus UI using [http://localhost:9090](http://localhost:9090).
Now you can access Prometheus UI. It is usually available at `http://localhost:9090`. For more details on exposing prometheus metrics, please [refer](https://prometheus.io/docs/prometheus/latest/getting_started/#starting-prometheus).

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

@assafad
Copy link
Contributor Author

assafad commented Mar 15, 2023

Hi @sradco, @everettraven, @varshaprasad96, anything else we're missing in order to merge this one?

@sradco
Copy link
Contributor

sradco commented Mar 20, 2023

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Mar 20, 2023

@sradco: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

varshaprasad96
varshaprasad96 approved these changes Mar 24, 2023
View reviewed changes
Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

The changes look good. @assafad thanks for the PR!. Could you please address this one comment, so that we can make the sanity pass before merging it.

@assafad assafad force-pushed the monitoring-docs branch 3 times, most recently from 0889ac0 to 2711ee4 Compare March 24, 2023 20:19
@assafad
Add metrics and alerts tutorial to the docs
d851f60 
Signed-off-by: assaf-admi <aadmi@redhat.com>
@varshaprasad96 varshaprasad96 merged commit 3af4acb into operator-framework:master Mar 27, 2023
oceanc80 pushed a commit to oceanc80/operator-sdk that referenced this pull request Apr 28, 2023
@assafad @oceanc80
Add metrics and alerts tutorial to the docs (operator-framework#6341)
bb50f2c 
Signed-off-by: assaf-admi <aadmi@redhat.com>
Signed-off-by: Catherine Chan-Tse <cchantse@redhat.com>
@oceanc80 oceanc80 mentioned this pull request Apr 28, 2023
Merged
oceanc80 added a commit that referenced this pull request Apr 28, 2023
@oceanc80 @assafad
Add metrics and alerts tutorial to the docs (#6341) (#6411)
a59326e 
Signed-off-by: assaf-admi <aadmi@redhat.com>
Signed-off-by: Catherine Chan-Tse <cchantse@redhat.com>
Co-authored-by: Assaf Admi <90143867+assafad@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sradco sradco sradco left review comments

@everettraven everettraven everettraven approved these changes

@varshaprasad96 varshaprasad96 varshaprasad96 approved these changes

@jberkhahn jberkhahn Awaiting requested review from jberkhahn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@assafad @sradco @everettraven @varshaprasad96