Skip to content

Add monitoring to OSDK #13666

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
Merged
merged 1 commit into from
Feb 22, 2019
Merged

Add monitoring to OSDK #13666

merged 1 commit into from
Feb 22, 2019

Conversation

adellape
Copy link
Contributor

@adellape adellape commented Feb 19, 2019 •
edited
Loading

@adellape adellape added this to the Future Release milestone Feb 19, 2019
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 19, 2019
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 19, 2019
@vikram-redhat
Copy link
Contributor

@rh-max can you review to make sure this is still correct?

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2019
@openshift-docs-preview-bot
Copy link

The preview will be availble shortly at:

@adellape adellape changed the title Add monitoring to OSDK [WIP] Add monitoring to OSDK Feb 20, 2019
adellape
adellape commented Feb 20, 2019
View reviewed changes
modules/osdk-monitoring-prometheus-servicemonitor-creating.adoc Outdated

.Procedure

. Add the `metrics.CreateServiceMonitor()` helper function, creating only one
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lilic What should this be added to?

Copy link

Choose a reason for hiding this comment

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

Wherever in the users operator code.

adellape
adellape commented Feb 20, 2019
View reviewed changes
modules/osdk-monitoring-prometheus-servicemonitor-creating.adoc Outdated
import "github.com/operator-framework/operator-sdk/pkg/metrics"

// Pass the Service(s) to the helper function, which in turn returns the `ServiceMonitor` object.
serviceMonitors, err := metrics.CreateServiceMonitors(restConfig, "namespace", services)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lilic Should "namespace" be a replaceable? services?

Copy link

Choose a reason for hiding this comment

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

services are meant in this case an array of kubernetesService objects the user creates for metrics exposing.

Yes "namespace" is a silly example I guess, should include the signature of the function as well. It's the name of the Namespace in which the ServiceMonitor will be created.

CreateServiceMonitors(config *rest.Config, ns string, services []*v1.Service)

@lilic
Copy link

lilic commented Feb 21, 2019

FYI I updated the original docs PR a bit.

@ahardin-rh ahardin-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Feb 22, 2019
@ahardin-rh
Copy link
Contributor

@adellape LGTM! 💯

@adellape
Copy link
Contributor Author

@lilic @ahardin-rh Thanks for the reviews! Updated with my latest understanding. Going to merge to get this in for QE review, and we can continue iterating from there.

Also cc @joelanford since you were reviewing the upstream operator-framework/operator-sdk#719 as well.

@adellape adellape merged commit 2d5a49b into openshift:enterprise-4.0 Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilic lilic lilic left review comments

Assignees
No one assigned
Labels
branch/enterprise-4.1 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 4.1 GA
Development

Successfully merging this pull request may close these issues.
6 participants
@adellape @vikram-redhat @openshift-docs-preview-bot @lilic @ahardin-rh @openshift-ci-robot