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

Correct telemetry for Prometheus doc #2866

Merged
merged 5 commits into from
Dec 19, 2018

Conversation

clyang82
Copy link
Member

@clyang82 clyang82 commented Nov 13, 2018

Signed-off-by: Chun Lin Yang [email protected]

Make the document (https://preliminary.istio.io/docs/tasks/telemetry/querying-metrics/) changes to reflect - istio/istio#8415

@istio-testing
Copy link
Contributor

Hi @clyang82. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@clyang82
Copy link
Member Author

/ @douglas-reid is my understanding right? please review. Thanks.

Copy link
Contributor

@douglas-reid douglas-reid left a comment

Choose a reason for hiding this comment

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

Thanks so much for tackling this item! I had a few nits/suggestions, but this is great.

to monitor Mixer itself.
1. *envoy* (`istio-mixer.istio-system:9102`): raw stats generated by Envoy (and
translated from Statsd to Prometheus).
1. *envoy* (`istio-proxy:15090`): raw stats generated by Envoy.
Copy link
Contributor

Choose a reason for hiding this comment

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

while we are here, can we also add entries for pilot, galley, and istio-policy (and update the "The configured Prometheus add-on scrapes three endpoints:" to read "The configured Prometheus add-on scrapes the following endpoints:") ?

Copy link
Member Author

Choose a reason for hiding this comment

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

added.


For more on querying Prometheus, please read their [querying
Prometheus configures to look for pods with the endpoint exposed and filters out a large number of envoy metrics based on scraping configuration. For more on querying Prometheus, please read their [querying
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "Prometheus is configured to look for pods with the envoy-prom endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data processed by the add-on."

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@clyang82
Copy link
Member Author

@douglas-reid Thanks for your suggestions. I have updated accordingly. I have one question about Prometheus? there are some configurations to define how to scrape the data. this is prometheus pull mode. In mixer adapter, there is a prometheus adapter which is using push mode. Is my understanding right? What is the difference in istio for these 2 modes? Thanks.

Copy link
Contributor

@rcaballeromx rcaballeromx left a comment

Choose a reason for hiding this comment

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

Do not use parenthesis; they are ambiguous in their meaning. Instead use verbs and prepositions to make the actions the reader can take and the results of those actions clear.


1. *istio-mesh* (`istio-mixer.istio-system:42422`): all Mixer-generated mesh
1. *istio-mesh* (`istio-telemetry.istio-system:42422`): all Mixer-generated mesh
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these not capitalized and separated. Is that the actual value of the endpoint? That case seems unlikely since I see the actual code in the parenthesis. Please capitalize appropriately and write them as separate words. Please use verbs and prepositions to make the actions explicit:

Suggested change
1. *istio-mesh* (`istio-telemetry.istio-system:42422`): all Mixer-generated mesh
1. **Istio mesh** addressable at `istio-telemetry.istio-system:42422`: Returns all Mixer-generated mesh

Copy link
Member Author

Choose a reason for hiding this comment

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

@rcaballeromx Thanks for your suggestions. actually, they are job names configured in prometheus. refer this document - https://preliminary.istio.io/help/ops/telemetry/missing-metrics/#verify-prometheus-configuration , you can find the job name

Copy link
Contributor

Choose a reason for hiding this comment

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

Then, please refer to them as jobs and link to that page in your content. For example: The istio-mesh job ...

Copy link
Member Author

@clyang82 clyang82 Nov 27, 2018

Choose a reason for hiding this comment

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

@rcaballeromx The page is used to tell the user how to verify the prometheus configurations. I do not think it is proper to link that here. How about change to - istio-telemetry.istio-system:42422: The istio-mesh job returns all Mixer-generated mesh

metrics.
1. *mixer* (`istio-mixer.istio-system:9093`): all Mixer-specific metrics. Used
1. *istio-telemetry* (`istio-telemetry.istio-system:9093`): all Mixer-specific metrics. Used
Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment on line 96:

Suggested change
1. *istio-telemetry* (`istio-telemetry.istio-system:9093`): all Mixer-specific metrics. Used
1. **Istio telemetry** addressable at `istio-telemetry.istio-system:9093`: Returns all Mixer-specific metrics. Use this endpoint

to monitor Mixer itself.
1. *envoy* (`istio-mixer.istio-system:9102`): raw stats generated by Envoy (and
translated from Statsd to Prometheus).
1. *envoy-stats* (`istio-proxy:15090`): raw stats generated by Envoy. Prometheus is configured to look for pods with the `envoy-prom` endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data processed by the add-on.
Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment on line 96:

Suggested change
1. *envoy-stats* (`istio-proxy:15090`): raw stats generated by Envoy. Prometheus is configured to look for pods with the `envoy-prom` endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data processed by the add-on.
1. Envoy stats addressable at `istio-proxy:15090`: Returns the raw stats generated by Envoy. Prometheus is configured to look for pods with the `envoy-prom` endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data the add-on processes.

1. *envoy* (`istio-mixer.istio-system:9102`): raw stats generated by Envoy (and
translated from Statsd to Prometheus).
1. *envoy-stats* (`istio-proxy:15090`): raw stats generated by Envoy. Prometheus is configured to look for pods with the `envoy-prom` endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data processed by the add-on.
1. *pilot* (`istio-pilot.istio-system:9093`): all pilot metrics.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. *pilot* (`istio-pilot.istio-system:9093`): all pilot metrics.
1. **Pilot** addressable at`istio-pilot.istio-system:9093`: Returns all the Pilot-generated metrics.

translated from Statsd to Prometheus).
1. *envoy-stats* (`istio-proxy:15090`): raw stats generated by Envoy. Prometheus is configured to look for pods with the `envoy-prom` endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data processed by the add-on.
1. *pilot* (`istio-pilot.istio-system:9093`): all pilot metrics.
1. *galley* (`istio-galley.istio-system:9093`): all galley metrics.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. *galley* (`istio-galley.istio-system:9093`): all galley metrics.
1. **Galley** addressable at `istio-galley.istio-system:9093`: Returns all the Galley-generated metrics.

1. *envoy-stats* (`istio-proxy:15090`): raw stats generated by Envoy. Prometheus is configured to look for pods with the `envoy-prom` endpoint exposed. The add-on configuration filters out a large number of envoy metrics during collection in an attempt to limit the scale of data processed by the add-on.
1. *pilot* (`istio-pilot.istio-system:9093`): all pilot metrics.
1. *galley* (`istio-galley.istio-system:9093`): all galley metrics.
1. *istio-policy* (`istio-policy.istio-system:9093`): all policy metrics.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. *istio-policy* (`istio-policy.istio-system:9093`): all policy metrics.
1. **Istio policy** addressable at `istio-policy.istio-system:9093`: Returns all policy-related metrics.

@geeknoid
Copy link
Contributor

Can we move this forward? Only a few grammatical fixes left...

Thanks.

Signed-off-by: Chun Lin Yang <[email protected]>
Signed-off-by: Chun Lin Yang <[email protected]>
Signed-off-by: Chun Lin Yang <[email protected]>
@clyang82 clyang82 force-pushed the fix_telemetry_prome branch from 9dfc292 to d03a298 Compare December 18, 2018 03:28
translated from Statsd to Prometheus).
The configured Prometheus add-on scrapes the following endpoints:

1. `istio-telemetry.istio-system:42422`: The `istio-mesh` job returns all Mixer-generated mesh.
Copy link
Contributor

Choose a reason for hiding this comment

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

mesh metrics

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Chun Lin Yang <[email protected]>
@gyliu513
Copy link
Member

/lgtm

@istio-testing
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: clyang82, gyliu513
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rcaballeromx

If they are not already assigned, you can assign the PR to them by writing /assign @rcaballeromx in a comment when ready.

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

@geeknoid geeknoid merged commit c0d784e into istio:master Dec 19, 2018
SataQiu pushed a commit to SataQiu/istio.io that referenced this pull request Jan 3, 2019
* Correct telemetry for prometheus doc

Signed-off-by: Chun Lin Yang <[email protected]>

* Add galley,polit and policy

Signed-off-by: Chun Lin Yang <[email protected]>

* update zh doc

Signed-off-by: Chun Lin Yang <[email protected]>

* Address review comments

Signed-off-by: Chun Lin Yang <[email protected]>

* mesh to metrics

Signed-off-by: Chun Lin Yang <[email protected]>
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.

7 participants