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

[exporter/tanzuobservability] Add metrics to README. #7948

Closed
wants to merge 1 commit into from

Conversation

keep94
Copy link
Contributor

@keep94 keep94 commented Feb 17, 2022

Documentation:
Add documentation for the metrics portion of the tanzu observability exporter.

@keep94 keep94 requested review from a team and codeboten February 17, 2022 01:17
Copy link
Contributor

@thepeterstone thepeterstone left a comment

Choose a reason for hiding this comment

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

Thanks Travis!

exporter/tanzuobservabilityexporter/README.md Outdated Show resolved Hide resolved
exporter/tanzuobservabilityexporter/README.md Outdated Show resolved Hide resolved
exporter/tanzuobservabilityexporter/README.md Outdated Show resolved Hide resolved
exporter/tanzuobservabilityexporter/README.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@codeboten codeboten added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 28, 2022
@@ -1,21 +1,29 @@
# Tanzu Observability (Wavefront) Exporter

This exporter supports sending traces to [Tanzu Observability](https://tanzu.vmware.com/observability).
This exporter supports sending both traces and metrics to [Tanzu Observability](https://tanzu.vmware.com/observability).

## Prerequisites

- [Obtain the Tanzu Observability by Wavefront API token.](https://docs.wavefront.com/wavefront_api.html#generating-an-api-token)
- [Set up and start a Tanzu Observability by Wavefront proxy](https://docs.wavefront.com/proxies_installing.html) and configure it with the API token you obtained.
- To have the proxy generate [span RED metrics](https://docs.wavefront.com/trace_data_details.html#red-metrics) from trace data, [configure](https://docs.wavefront.com/proxies_configuring.html) the proxy's `customTracingListenerPorts` and use it for the exporter's endpoint.
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs updating now that we support metrics in addition to traces, and should be simplified. Let's replace the bullet point above with:

  • Configure the proxy to receive traces by setting customTracingListenerPorts=30001. For metrics, the proxy listens on port 2878 by default.

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.

exporter/tanzuobservabilityexporter/README.md Outdated Show resolved Hide resolved
exporter/tanzuobservabilityexporter/README.md Outdated Show resolved Hide resolved
- Delta histograms get converted to histograms in wavefront
- Exponential histograms are converted the same way as regular histograms
- Each summary metric is converted to multiple gauge metrics (one gauge metric for each quantile in the summary). A special "quantile" tag contains a value between 0 and 1 indicating the quantile for which the value belongs.

## Tanzu Observability Specific Attributes

Copy link
Member

Choose a reason for hiding this comment

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

Let's replace this and the two sub-bullets with:

  • Tanzu Observability Spans require two Application Tags: application and service. If missing, the exporter defaults them to application=defaultApp and service=defaultService.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, my comment doesn't account for service.name. We'll pair up to figure out the wording.

CHANGELOG.md Outdated Show resolved Hide resolved
@@ -47,13 +55,19 @@ exporters:
tanzuobservability:
Copy link
Member

@oppegard oppegard Mar 4, 2022

Choose a reason for hiding this comment

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

When Travis and I pair on updating this PR, let's consider promoting the Configuration section towards the top of the README, since that's likely what users care about most to get the exporter running.

@keep94
Copy link
Contributor Author

keep94 commented Mar 10, 2022

FYI, this PR now depends on PR #8338

@keep94 keep94 marked this pull request as draft March 10, 2022 23:25
Copy link
Member

@oppegard oppegard left a comment

Choose a reason for hiding this comment

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

I added comments – but I've addressed them in the commit I pushed to reorganize the readme.

Given a Wavefront proxy at `10.10.10.10`, configured with `customTracingListenerPorts` set to `30001`, the traces endpoint would be `http://10.10.10.10:30001`
and the metrics endpoint would be `http://10.10.10.10:2878`.

### Resource Attributes
Copy link
Member

Choose a reason for hiding this comment

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

Let's title this "Resource Attributes on Metrics".

Copy link
Member

Choose a reason for hiding this comment

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

Let's also move this section below the "Example Configuration" section.

@@ -78,7 +110,8 @@ exporters:
endpoint: "http://10.10.10.10:30001"
metrics:
endpoint: "http://10.10.10.10:2878"
resource_attributes: false
resource_attributes:
enabled: true
Copy link
Member

Choose a reason for hiding this comment

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

Let's set this to false in the likely event someone copy and pastes this example when getting started.

Comment on lines 90 to 94
Client programs can send a series of global key-value pairs along with metrics to the
open telemetry collector. By default, the Tanzu Observability Exporter does not send these, but
if the resource_attributes stanza is included in the configuration file as it is below, the
Tanzu Observability Exporter will include sent resource attributes as additional tags in each
metric.
Copy link
Member

Choose a reason for hiding this comment

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

Slight wording and formatting tweaks to this:

Client programs using an OpenTelemetry SDK can be configured to wrap all emitted telemetry (metrics, spans, logs) with a set of global key-value pairs, called resource attributes. By default, the Tanzu Observability Exporter includes resource attributes on spans but excludes them on metrics. To include resource attributes as tags on metrics, configure the resource_attributes stanza per the example above.

Comment on lines 82 to 83
The required configurations are two Wavefront proxy API endpoints: one to receive traces and one to
receive metrics from the Tanzu Observability Exporter.
Copy link
Member

Choose a reason for hiding this comment

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

Let's delete this whole sentence.

@keep94 keep94 marked this pull request as ready for review March 24, 2022 16:22
@jpkrohling
Copy link
Member

Once this has been reviewed by code owners and is ready for review by me, let me know.

Copy link
Member

@oppegard oppegard left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@keep94 keep94 marked this pull request as draft April 5, 2022 21:50
@keep94
Copy link
Contributor Author

keep94 commented Apr 11, 2022

Hi, this change has become a part of PR #9098

@keep94 keep94 closed this Apr 11, 2022
@keep94 keep94 deleted the readme_changes branch April 11, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants