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

Improve OC Receiver Documentation #203

Merged
merged 9 commits into from
Aug 1, 2019
Merged

Improve OC Receiver Documentation #203

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
96211b4
First round of receiver and opencensus receiver documentation.
ccaraman Jul 29, 2019
4a0c43f
Merge branch 'master' into receiver_oc_getting_started
ccaraman Jul 29, 2019
c6b11e0
Undo go mod/sum changes
ccaraman Jul 29, 2019
54e334c
Address initial set of comments.
ccaraman Jul 29, 2019
bad4b8b
Address next set of comments.
ccaraman Jul 30, 2019
7b1d161
Address next set of comments.
ccaraman Jul 30, 2019
048068a
Fix use of server instead of receiver in comment and explain settings…
ccaraman Aug 1, 2019
c8d8d89
Merge branch 'master' into receiver_oc_getting_started
ccaraman Aug 1, 2019
dbfbc7f
Merged master and fixed mispell error caugh with new tools
ccaraman Aug 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 39 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,27 @@ exporting will resume.
## <a name="config"></a>Configuration

The OpenTelemetry Service (both the Agent and Collector) is configured via a
YAML file. In general, you need to configure one or more receivers as well as
one or more exporters. In addition, diagnostics can also be configured.
YAML file. In general, at least one enabled receiver and one exporter needs to be configured.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
YAML file. In general, at least one enabled receiver and one exporter needs to be configured.
YAML file. In general, at least one enabled receiver and one enabled exporter needs to be configured.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Updating!


*Note* This documentation is still in progress. For any questions, please reach out in the
Copy link
Member

Choose a reason for hiding this comment

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

👍

[OpenTelemetry Gitter](https://gitter.im/open-telemetry/opentelemetry-service) or
refer to the [issues page](https://github.com/open-telemetry/opentelemetry-service/issues).

There are four main parts to a config:
```yaml
receivers:
...
exporters:
...
processors:
...
pipelines:
...
```

### <a name="config-receivers"></a>Receivers

A receiver is how you get data into the OpenTelemetry Service. One or more
receivers can be configured. By default, the `opentelemetry` receiver is enabled
on the Collector and required as a defined receiver for the Agent.
A receiver is how data gets into OpenTelemetry Service. One or more receivers must be configured.

A basic example of all available receivers is provided below. For detailed
receiver configuration, please see the [receiver
Expand Down Expand Up @@ -181,6 +194,25 @@ exporters:
endpoint: "http://127.0.0.1:9411/api/v2/spans"
```

### <a name="config-pipelines"></a>Pipelines
Pipelines can be of two types:

- metrics: collects and processes metrics data.
- traces: collects and processes trace data.

A pipeline consists of a set of receivers, processors, and exporters. Each receiver/processor/exporter must be specified
Copy link
Member

Choose a reason for hiding this comment

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

Formatting suggestion: either stick to soft line breaks (i.e. only do hard line breaks between paragraphs) or use hard line breaks at around 80 characters in MD files. With longer lines (like 120 chars in this case) the side-by-side diff in Github doesn't look as nice unless you have a very large monitor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good - I'll update.

To make things easier to catch especially with formatting, can we add a md formatter to catch all of these things?

Copy link
Member

Choose a reason for hiding this comment

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

There is an ongoing discussion on this topic: open-telemetry/opentelemetry-specification#192

After the decision is made we can look into having an md formatter to enforce the rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! That will be helpful to save everyone time and even have it as a precommit check!

in the configuration to be included in a pipeline and each receiver/processor/exporter can be used in more than one pipeline.
Copy link
Member

Choose a reason for hiding this comment

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

There is a slight nuance with processors that is worth mentioning explicitly.

When the same processor is referenced in multiple pipelines each pipelines gets a separate instance of that processor (each instance having the same config but separate internal state). By contrast receivers and exporters referenced from multiple pipelines are the same instance of the receiver and exporter.

So, for example if I have a "queue" processor referenced from 2 pipelines, it really means 2 separate queues, one for each pipeline.

While if I reference "OpenCensus" receiver from 2 pipeline, it is really the same receiver and the data it receives is duplicated and fed into 2 pipelines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good distinction that should be added. I will add it in this pr.
As the description said, this document change is only a starting point and I'm sure there is much missing from it. Once we have a good skeleton in place (probably needs another pr or two to get it solid), we can fill in remaining gaps.


The following is an example pipeline configuration. For more information, refer to [processor README.md](processor/README.md)
```yaml
pipelines:
traces/first:
Copy link
Member

Choose a reason for hiding this comment

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

The traces/first key syntax is a) slightly unconventional b) has meaning that is not immediately obvious. It is worth explaining the "type/name" key convention somewhere close to this place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove '/first' and keep the example simple. This documentation will need a few more prs to make it a good landing page.

receivers: [examplereceiver]
processors: [exampleprocessor]
exporters: [exampleexporter]

```

### <a name="config-diagnostics"></a>Diagnostics

zPages is provided for monitoring running by default on port ``55679``.
Expand All @@ -207,7 +239,9 @@ zpages:
disabled: true
```


### <a name="global-attributes"></a> Global Attributes
**TODO** Remove this once processors have been documented since that handles these features now.

The OpenTelemetry Service also takes some global configurations that modify its
behavior for all receivers / exporters. This configuration is typically applied
Expand Down Expand Up @@ -300,7 +334,6 @@ sampling:
```

> Note that an exporter can only have a single sampling policy today.

## <a name="collector-usage"></a>Usage

> It is recommended that you use the latest [release](https://github.com/open-telemetry/opentelemetry-service/releases).
Expand Down
30 changes: 30 additions & 0 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ const (
errPipelineExporterNotExists
errMetricPipelineCannotHaveProcessors
errUnmarshalError
errMissingReceivers
ccaraman marked this conversation as resolved.
Show resolved Hide resolved
// TODO(ccaraman): Add an error for missing Processors with corresponding test cases.
)

type configError struct {
Expand Down Expand Up @@ -175,6 +177,19 @@ func loadReceivers(v *viper.Viper, factories map[string]receiver.Factory) (confi
// Get the map of "receivers" sub-keys.
keyMap := v.GetStringMap(receiversKeyName)

// Currently there is no default receiver enabled. The configuration must specify at least one receiver to enable
Copy link
Member

Choose a reason for hiding this comment

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

We have separate validation code. Would it make sense to move this code there?

Also, I would suggest to split this PR into 2 parts: code change and documentation change. It is difficult to review both at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to add a todo to the validateReceivers that we need to revisit the approach of deleting the disabled receivers because it sounds like there will be a cli way to enable/disable a receiver.

One reason why the validateReceivers need to happen later on in the process of loading a configuration is loading pipelines checks if a receiver is enabled or not before adding it to its configuration so we can't remove the disabled receivers in this part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re splitting this pr into 2 -are you okay with doing it for future prs to keep the history of the conversations?

Copy link
Member

Choose a reason for hiding this comment

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

I highly recommend to split the PR. You do not need to delete this one, just remove changes that are not relevant to documenting existing behavior.

One PR and one commit per concern is much nicer. This PR is performing 2 mostly independent changes, I do not see good reasons to group them. I'd just remove config validation code changes from this PR and keep everything about documentation and tests which are related to documentation.

Let this be a separate PR:

Add validation/test to have at least one enabled receiver in configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced offline - This will be a one off and leave all of the changes together since i ended up fixing code functionality that was related to fixing the docs (ex: ensuring that there is one enabled receiver was mentioned but not enforced in code with no test case either)

// functionality.
if len(keyMap) == 0 {
return nil, &configError{
code: errMissingReceivers,
msg: "no receivers specified in config",
}
}

// This boolean is used to track if there are any enabled receivers. If there are none at the end of loading
// all of the receivers, throw an error.
enabledReceiver := false

// Prepare resulting map
receivers := make(configmodels.Receivers, 0)

Expand Down Expand Up @@ -211,6 +226,9 @@ func loadReceivers(v *viper.Viper, factories map[string]receiver.Factory) (confi
err = customUnmarshaler(subViper, key, receiverCfg)
} else {
// Standard viper unmarshaler is fine.
// TODO(ccaraman): UnmarshallExact should be used to catch erroneous config entries.
// This leads to quickly identifying config values that are not supported and reduce confusion for
// users.
err = subViper.UnmarshalKey(key, receiverCfg)
}

Expand All @@ -228,9 +246,20 @@ func loadReceivers(v *viper.Viper, factories map[string]receiver.Factory) (confi
}
}

// Or'ing the enabled flag for all receivers will return true if at least one is enabled.
enabledReceiver = enabledReceiver || receiverCfg.IsEnabled()

receivers[fullName] = receiverCfg
}

// There must be at least one enabled receiver for the config to be valid.
if !enabledReceiver {
return nil, &configError{
code: errMissingReceivers,
msg: "no enabled receivers specified in config",
}
}

return receivers, nil
}

Expand Down Expand Up @@ -496,6 +525,7 @@ func validatePipelineReceivers(
zap.String("receiver", ref))
}
}

pipeline.Receivers = rs

return nil
Expand Down
3 changes: 2 additions & 1 deletion config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ func TestDecodeConfig_Invalid(t *testing.T) {
}{
{name: "empty-config"},
{name: "missing-all-sections"},
{name: "missing-receivers"},
{name: "missing-enabled-receivers", expected: errMissingReceivers},
Copy link
Member

Choose a reason for hiding this comment

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

👍

{name: "missing-receivers", expected: errMissingReceivers},
Copy link
Member

Choose a reason for hiding this comment

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

👍

{name: "missing-exporters"},
{name: "missing-processors"},
{name: "invalid-receiver-name"},
Expand Down
18 changes: 13 additions & 5 deletions config/configmodels/configmodels.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,19 @@ const (
// ReceiverSettings defines common settings for a single-protocol receiver configuration.
// Specific receivers can embed this struct and extend it with more fields if needed.
type ReceiverSettings struct {
TypeVal string `mapstructure:"-"`
NameVal string `mapstructure:"-"`
Disabled bool `mapstructure:"disabled"`
Endpoint string `mapstructure:"endpoint"`
DisableBackPressure bool `mapstructure:"disable-backpressure"`
TypeVal string `mapstructure:"-"`
NameVal string `mapstructure:"-"`
// Configures if the receiver is disabled and doesn't receive any data.
// The default value is false, and it is expected that receivers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// The default value is false, and it is expected that receivers
// The default value is false (meaning that the receiver is enabled by default), and it is expected that receivers

// continue to use the default value of false.
Disabled bool `mapstructure:"disabled"`
// Configures the endpoint in the format 'address:port' for the receiver.
// The default value is set by the receiver populating the struct.
Copy link
Member

Choose a reason for hiding this comment

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

👍

Endpoint string `mapstructure:"endpoint"`
// Configures if enabling the back pressure functionality.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Configures if enabling the back pressure functionality.
// Configures if the back pressure functionality is disabled for this receiver.

// The default value is false, and it is expected that receivers
// continue to use the default value of false.
DisableBackPressure bool `mapstructure:"disable-backpressure"`
}

// Name gets the receiver name.
Expand Down
23 changes: 23 additions & 0 deletions config/testdata/missing-enabled-receivers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
receivers:
examplereceiver:
disabled: true
examplereceiver/disabled:
disabled: true

processors:
exampleprocessor:
exampleprocessor/disabled:
disabled: true

exporters:
exampleexporter/myexporter:
extra: "some export string 2"
exampleexporter/disabled:
disabled: true
exampleexporter:

pipelines:
traces:
receivers: [examplereceiver, examplereceiver/disabled]
processors: [exampleprocessor, exampleprocessor/disabled]
exporters: [exampleexporter/disabled, exampleexporter]
1 change: 1 addition & 0 deletions internal/zpagesserver/zpagesserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

const (
// ZPagesHTTPPort is the name of the flag used to specify the zpages port.
// TODO(ccaraman): Move ZPage configuration to be apart of global config/config.go, maybe under diagnostics section.
ZPagesHTTPPort = "zpages-http-port"
)

Expand Down
4 changes: 4 additions & 0 deletions processor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Processor
*Note* This documentation is still in progress. For any questions, please reach out in the
[OpenTelemetry Gitter](https://gitter.im/open-telemetry/opentelemetry-service) or
refer to the [issues page](https://github.com/open-telemetry/opentelemetry-service/issues).
98 changes: 66 additions & 32 deletions receiver/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
A variety of receivers are available to the OpenCensus Service (both Agent and Collector)
**Note** This documentation is still in progress. For any questions, please reach out in the
[OpenTelemetry Gitter](https://gitter.im/open-telemetry/opentelemetry-service) or
refer to the [issues page](https://github.com/open-telemetry/opentelemetry-service/issues).

__Currently there are some inconsistencies between Agent and Collector configuration, those will be addressed by issue
[#135](https://github.com/census-instrumentation/opencensus-service/issues/135).__
# Receivers
A receiver is how data gets into OpenTelemetry Service. Generally, a receiver accepts data in a specified format and can
support [traces](https://github.com/open-telemetry/opentelemetry-proto/blob/2ccb7cb0cf038086955c46eebea767fddb331d16/src/opentelemetry/proto/trace/v1/trace.proto) and/or [metrics](https://github.com/open-telemetry/opentelemetry-proto/blob/2ccb7cb0cf038086955c46eebea767fddb331d16/src/opentelemetry/proto/metrics/v1/metrics.proto).
Copy link
Member

Choose a reason for hiding this comment

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

Unclear why this links to a specific commit of draft OpenTelemetry proto repo. A trace can come from non-OpenTelemetry protocols too.

I would like to generic definition of a trace (if a link is needed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gah - another example of gh convo's being shrunk/moved after a change!
This is the first set of 'docs' I could find to talk about traces/metrics with the help of the reviewers of the pr. Is there a document that talks about traces/metrics at a higher level and that isn't otel specific?

Copy link
Member

Choose a reason for hiding this comment

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

I am not aware of a document outside otel that is useful to link from here. Inside otel perhaps check the specification repo and see if any of the documents there is appropriate for linking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm - i can't find anything either. There needs to be somewhere that defines what is meant by traces/metrics. I'll remove the hyperlinks for now.


## OpenCensus

This receiver receives spans from OpenCensus instrumented applications and translates them into the internal span types that are then sent to the collector/exporters.
Supported receivers (sorted alphabetically):
- [Jaeger Receiver](#jaeger)
- [OpenCensus Receiver](#opencensus)
- [Prometheus Receiver](#prometheus)
- [VM Metrics Receiver](#vmmetrics)
- [Zipkin Receiver](#zipkin)

## Configuring Receiver(s)
TODO - Add what a fullname is and how that is referenced in other parts of the configuration.
Describe the common receiver settings: endpoint, disabled and disable-backpressure.

## <a name="opencensus"></a>OpenCensus Receiver
**Traces and metrics are supported.**

This receiver receives spans from [OpenCensus](https://opencensus.io/) instrumented applications and translates them into the internal format sent to processors and exporters in the pipeline.

Its address can be configured in the YAML configuration file under section "receivers", subsection "opencensus" and field "address". The syntax of the field "address" is `[address|host]:<port-number>`.

Expand Down Expand Up @@ -39,7 +55,12 @@ receivers:
- https://*.example.com
```

### Deprecated YAML Configurations
**Note**: This isn't a full list of deprecated OpenCensus YAML configurations. If something is missing, please expand the documentation
or open an issue.

### Collector Differences
TODO(ccaraman) - Delete all references to opencensus-service issue 135 in follow up pr.
(To be fixed via [#135](https://github.com/census-instrumentation/opencensus-service/issues/135))

By default this receiver is ALWAYS started on the OpenCensus Collector, it can be disabled via command-line by
Expand Down Expand Up @@ -97,9 +118,12 @@ receivers:
permit-without-stream: true
```

## Jaeger

## <a name="jaeger"></a>Jaeger Receiver
**Only traces are supported.**

This receiver receives spans from Jaeger collector HTTP and Thrift uploads and translates them into the internal span types that are then sent to the collector/exporters.
Only traces are supported. This receiver does not support metrics.

Its address can be configured in the YAML configuration file under section "receivers", subsection "jaeger" and fields "collector_http_port", "collector_thrift_port".

Expand All @@ -124,32 +148,8 @@ receivers:
jaeger-thrift-http-port: 14268
```

## Zipkin

This receiver receives spans from Zipkin (V1 and V2) HTTP uploads and translates them into the internal span types that are then sent to the collector/exporters.

Its address can be configured in the YAML configuration file under section "receivers", subsection "zipkin" and field "address". The syntax of the field "address" is `[address|host]:<port-number>`.

For example:

```yaml
receivers:
zipkin:
address: "127.0.0.1:9411"
```

### Collector Differences
(To be fixed via [#135](https://github.com/census-instrumentation/opencensus-service/issues/135))

On the Collector Zipkin reception at the port 9411 can be enabled via command-line `--receive-zipkin`. On the Collector only the port can be configured, example:

```yaml
receivers:
zipkin:
port: 9411
```

## Prometheus
## <a name="prometheus"></a>Prometheus Receiver
**Only metrics are supported.**

This receiver is a drop-in replacement for getting Prometheus to scrape your services. Just like you would write in a
YAML configuration file before starting Prometheus, such as with:
Expand Down Expand Up @@ -180,3 +180,37 @@ receivers:
static_configs:
- targets: ['localhost:9777']
```

## <a name="vmmetrics"></a>VM Metrics Receiver
**Only metrics are supported.**

<Add more information - I'm lonely.>

## <a name="zipkin"></a>Zipkin Receiver
**Only traces are supported.**

This receiver receives spans from Zipkin (V1 and V2) HTTP uploads and translates them into the internal span types that are then sent to the collector/exporters.
pjanotti marked this conversation as resolved.
Show resolved Hide resolved

Its address can be configured in the YAML configuration file under section "receivers", subsection "zipkin" and field "address". The syntax of the field "address" is `[address|host]:<port-number>`.

For example:

```yaml
receivers:
zipkin:
address: "127.0.0.1:9411"
tigrannajaryan marked this conversation as resolved.
Show resolved Hide resolved
```

### Collector Differences
pjanotti marked this conversation as resolved.
Show resolved Hide resolved
(To be fixed via [#135](https://github.com/census-instrumentation/opencensus-service/issues/135))

On the Collector Zipkin reception at the port 9411 can be enabled via command-line `--receive-zipkin`. On the Collector only the port can be configured, example:

```yaml
receivers:
zipkin:
port: 9411
```

## Common Configuration Errors
<Fill this in as we go with common gotchas experienced by users. These should eventually be made apart of the validation test suite.>
Loading