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 doc to setup Prometheus via operator and remove ref of manager_p… #1245

Merged
merged 1 commit into from Jan 15, 2020

Conversation

@camilamacedo86
Copy link
Member

camilamacedo86 commented Dec 6, 2019

Closes: #1244

@k8s-ci-robot k8s-ci-robot requested review from Liujingfang1 and pwittrock Dec 6, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch from c72d75c to c61d959 Dec 6, 2019
@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Dec 6, 2019

/assign @mengqiy

Could you please help us here? Please, let me know wdyt about the explanation and if you found any other place that also we need to update.

@christopherhein

This comment has been minimized.

Copy link
Member

christopherhein commented Dec 6, 2019

WDYT about moving the exporting to after it talks about how to use the sigs.k8s.io/controller-runtime/pkg/metrics to export additional metrics?

As a controller builder I'm much more interested in how to use the built-in metrics functions to add more than to immediately export the base, similar to how we talk about deployment and packaging later in the Table of Contents then implementing the controllers… thoughts?

@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Dec 6, 2019

Hi @christopherhein,

tks for your input.

WDYT about moving the exporting to after it talks about how to use the sigs.k8s.io/controller-runtime/pkg/metrics to export additional metrics?

IHMO would make more sense first explain how to export the metrics then, after that how to customize it as it was done.

docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch from c61d959 to 8d0a923 Dec 7, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch from 1241d4f to ecceb21 Dec 9, 2019
@k8s-ci-robot k8s-ci-robot added size/M and removed size/L labels Dec 9, 2019
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch 2 times, most recently from f2db88c to a21b05a Dec 10, 2019
@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Dec 10, 2019

/retest

@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Jan 6, 2020

H @mengqiy,

I think this one has all comments addressed and could be merged. Please, let me know if has anything else that should be done here.

@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch from a21b05a to 7602e06 Jan 10, 2020

# Value of this field is prepended to the
# names of all resources, e.g. a deployment named
# "wordpress" becomes "alices-wordpress".
# Note that it should also match with the prefix (text before '-') of the namespace
# field above.
namePrefix: project-
namePrefix: project-v2-

This comment has been minimized.

Copy link
@mengqiy

mengqiy Jan 14, 2020

Contributor

Same here.

testdata/project-v2/config/default/kustomization.yaml Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch from 7602e06 to e4ff120 Jan 14, 2020
@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Jan 14, 2020
Copy link
Contributor

Adirio left a comment

Suggestions to improve clearness and readability.

docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
docs/book/src/reference/metrics.md Outdated Show resolved Hide resolved
pkg/scaffold/v2/kustomize.go Outdated Show resolved Hide resolved
testdata/project-v2/config/default/kustomization.yaml Outdated Show resolved Hide resolved
…ometheus_metrics_patch.yaml which is no longer used in v2
@camilamacedo86 camilamacedo86 force-pushed the camilamacedo86:prome-doc branch from e4ff120 to 5b31a7e Jan 14, 2020
@camilamacedo86 camilamacedo86 requested a review from mengqiy Jan 14, 2020
@camilamacedo86

This comment has been minimized.

Copy link
Member Author

camilamacedo86 commented Jan 14, 2020

Hi @Adirio and @mengqiy,

Tks for the collab and reviews. all addressed I think it is a good state to be merged now.

@Adirio

This comment has been minimized.

Copy link
Contributor

Adirio commented Jan 14, 2020

All the conversations with my suggested changes can be marked as resolved.

LGTM

@mengqiy

This comment has been minimized.

Copy link
Contributor

mengqiy commented Jan 15, 2020

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 15, 2020
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 15, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, mengqiy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 247d268 into kubernetes-sigs:master Jan 15, 2020
7 of 8 checks passed
7 of 8 checks passed
tide Not mergeable.
Details
cla/linuxfoundation camilamacedo86 authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
netlify/kubebuilder/deploy-preview Deploy preview ready!
Details
pull-kubebuilder-e2e-k8s-1-14-1 Job succeeded.
Details
pull-kubebuilder-e2e-k8s-1-15-3 Job succeeded.
Details
pull-kubebuilder-e2e-k8s-1-16-2 Job succeeded.
Details
pull-kubebuilder-test Job succeeded.
Details
@camilamacedo86 camilamacedo86 deleted the camilamacedo86:prome-doc branch Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.