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

Update collecting-metrics.md #5717

Closed
wants to merge 2 commits into from
Closed

Update collecting-metrics.md #5717

wants to merge 2 commits into from

Conversation

pawarpranav83
Copy link

@pawarpranav83 pawarpranav83 commented Oct 13, 2023 •
edited
Loading

Editing the config map with specified commands.
Issue #5217.

"Fixes #issue-number" or "Add description of the problem this PR solves"

Proposed Changes

  • Addition of edit config map commands.

@pawarpranav83
Update collecting-metrics.md
8f41fa1 
Editing the config map with specified commands.
Issue knative#5217.
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 13, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@netlify
Copy link

netlify bot commented Oct 13, 2023 •
edited
Loading

✅ Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ba0fd64
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/65296149ed99ca0008a9b432
😎 Deploy Preview https://deploy-preview-5717--knative.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@knative-prow knative-prow bot requested review from nainaz and snneji October 13, 2023 15:12
@knative-prow
Copy link

knative-prow bot commented Oct 13, 2023

Welcome @pawarpranav83! It looks like this is your first PR to knative/docs 🎉

@knative-prow knative-prow bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 13, 2023
@knative-prow
Copy link

knative-prow bot commented Oct 13, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pawarpranav83
Once this PR has been reviewed and has the lgtm label, please assign dsimansk for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@knative-prow knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 13, 2023
@ReToCode
Copy link
Member

/assign @skonto can you take a look?

yanggangtony
yanggangtony reviewed Dec 5, 2023
View reviewed changes
docs/snippets/collecting-metrics.md
```bash
kubectl patch --namespace knative-serving configmap/config-observability \
--type merge \
--patch '{"data":{"metrics.backend-destination":"promethuus","metrics.request-metrics-backend-destination":"prometheus"}}'
Copy link

Choose a reason for hiding this comment

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

same key with under line.

yanggangtony
yanggangtony reviewed Dec 5, 2023
View reviewed changes
docs/snippets/collecting-metrics.md

kubectl patch --namespace knative-serving configmap/config-observability \
--type merge \
--patch '{"data":{"metrics.backend-destination":"promethuus","metrics.request-metrics-backend-destination":"prometheus"}}'
Copy link

Choose a reason for hiding this comment

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

same key with upper line.

@yanggangtony
Copy link

I think you should patch
metrics.backend-destination and metrics.request-metrics-backend-destination

@skonto
Copy link
Contributor

skonto commented Dec 13, 2023 •
edited
Loading

Hi @pawarpranav83, I can't reproduce the issue in #5217.
The default backend is Prometheus also stated in the docs, so there is no reason to set it explicitly. Btw I recently tested this with knative-extensions/monitoring#25, without setting any backend:

$ kubectl port-forward service/activator-service -n knative-serving 9090:9090
Forwarding from 127.0.0.1:9090 -> 9090
Forwarding from [::1]:9090 -> 9090
Handling connection for 9090
Handling connection for 9090

Btw also the preview does not look good.

@pawarpranav83 pawarpranav83 deleted the patch-1 branch January 6, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanggangtony yanggangtony yanggangtony left review comments

@nainaz nainaz Awaiting requested review from nainaz

@snneji snneji Awaiting requested review from snneji

Assignees
No one assigned
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pawarpranav83 @ReToCode @yanggangtony @skonto