Add Google Pubsub as an OTLP exporter/receiver #1892

alexvanboxel · 2020-12-22T20:25:30Z

Description:
Allows export and reception of OTLP data over Google Pubsub. With the ability to set subscriptions and topics for each of the components.

Link to tracking Issue:
#1802

Testing:
Unit tests are included

Documentation:
The documentation includes a short description and the configuration.

codecov · 2020-12-22T20:38:04Z

Codecov Report

Merging #1892 (bc8c7fd) into main (9677f5b) will decrease coverage by 0.66%.
The diff coverage is 99.05%.

@@            Coverage Diff             @@
##             main    #1892      +/-   ##
==========================================
- Coverage   90.70%   90.04%   -0.67%     
==========================================
  Files         399      385      -14     
  Lines       19802    18730    -1072     
==========================================
- Hits        17961    16865    -1096     
- Misses       1381     1394      +13     
- Partials      460      471      +11

Flag	Coverage Δ
integration	`69.73% <0.00%> (+0.46%)`	⬆️
unit	`88.77% <99.05%> (-0.74%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
receiver/gcloudpubsubreceiver/receiver.go	`97.18% <97.18%> (ø)`
cmd/otelcontribcol/components.go	`86.48% <100.00%> (-0.53%)`	⬇️
exporter/gcloudpubsubexporter/exporter.go	`100.00% <100.00%> (ø)`
exporter/gcloudpubsubexporter/factory.go	`100.00% <100.00%> (ø)`
receiver/gcloudpubsubreceiver/factory.go	`100.00% <100.00%> (ø)`
exporter/datadogexporter/metrics_exporter.go	`45.45% <0.00%> (-54.55%)`	⬇️
receiver/redisreceiver/redis_metric.go	`63.15% <0.00%> (-36.85%)`	⬇️
exporter/sumologicexporter/exporter.go	`50.00% <0.00%> (-31.92%)`	⬇️
exporter/datadogexporter/metadata/metadata.go	`60.78% <0.00%> (-28.93%)`	⬇️
exporter/datadogexporter/metadata/ec2/ec2.go	`30.00% <0.00%> (-17.50%)`	⬇️
... and 92 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9677f5b...85ce288. Read the comment docs.

alexvanboxel · 2020-12-23T20:20:19Z

I'm going to split this PR up in different PR's as the CONTRIBUTING suggests. I'll keep this one open for a while.

anuraaga

Did an initial skim, in general looks good. Do you mind splitting this up per these guidelines for a new component?

https://github.com/open-telemetry/opentelemetry-collector/blob/master/CONTRIBUTING.md#when-adding-a-new-component

anuraaga · 2020-12-24T02:48:08Z

exporter/gcloudpubsubexporter/config.go

+
+type Config struct {
+	configmodels.ExporterSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
+	ProjectID                     string                   `mapstructure:"project"`


Let's add some comments. Endpoint in particular needs something on when it would be set (probably very rarely by normal users)

anuraaga · 2020-12-24T02:49:55Z

exporter/gcloudpubsubexporter/README.md

+The following configuration options are supported:
+
+* `project` (Required): The Google Cloud Project of the topics.
+* `validate_existence`(Optional): Checks the existence of the topic, but this requires admin permissions to validate 


It would be good to list out the actual permission names required, both for normal use and when setting this flag.

Also I would consider removing this flag as it might just complicate things and failing when sending messages isn't that bad, many of our exporters work that way.

This check is interesting. This verification could make sense to be implemented on exporter startup (how you implemented it), as this would have a fast feedback loop and prevent trying to send data that is going to fail. However, topics can be created at anytime. Since the collector uses a static config, would it be safe to assume that the topic name is known and has been created prior to configuring a collector? Or is it best to assume a fully asynchronous approach and assume a topic may exist at some time in the future?

anuraaga · 2020-12-24T02:51:34Z

exporter/gcloudpubsubexporter/exporter.go

+
+func (ex *pubsubExporter) ConsumeTraces(ctx context.Context, td pdata.Traces) error {
+	bytes, _ := td.ToOtlpProtoBytes()
+	message := &pubsub.Message{Data: bytes}


Do you think we should put a PR in for the opentelemetry-specification to document how OTLP interacts with Pub/Sub? While it's straightforward to push the bytes into Data, there could still be different interpretations of how Pub/Sub is supposed to work so we probably want to make sure they're unified across implementations.

Thanks @anuraaga. I'll make one comment in reply to your initial review. Thanks, I needed a second eye on this.

I will add it to the spec. I'll ask in gitter to see if I should do a PR or an OTEP first. I see that Kafka does it the same way as I (sending the Request), so that's good.

I will split up the PR but will keep this one open in the meantime (I converted it to draft), I want to have someone from Google involved todo a quick scan.

All fix your concerns.

Looking forward to reviewing the spec you draft up, @alexvanboxel! 📘

anuraaga · 2020-12-24T02:53:08Z

exporter/gcloudpubsubexporter/README.md

+
+This exporter sends OTLP messages to a Google Cloud [Pubsub](https://cloud.google.com/pubsub) topic.
+
+The following configuration options are supported:


Missing documentation for user agent

fbogsany · 2021-01-06T15:09:21Z

exporter/gcloudpubsubexporter/exporter.go

+	"context"
+	"fmt"
+
+	"cloud.google.com/go/pubsub"


Shopify has been running a Google PubSub (trace) exporter in production for almost a year. The largest improvement in stability came from switching to the lower level package cloud.google.com/go/pubsub/apiv1. The high level package used here provides an asynchronous Publish method that does not allow back pressure in the collector, and limits the usefulness of the memorylimiter processor, queued retry, etc. The lower level package provides a synchronous Publish call, which is more appropriate for the collector.

cc @ibawt

Thanks, @fbogsany I will certainly rewrite this as I was a bit struggling with this. I'm writing the OTEP now and will refer to this in this thread.

Shopify has been running a Google PubSub (trace) exporter in production for almost a year. The largest improvement in stability came from switching to the lower level package cloud.google.com/go/pubsub/apiv1. The high level package used here provides an asynchronous Publish method that does not allow back pressure in the collector, and limits the usefulness of the memorylimiter processor, queued retry, etc. The lower level package provides a synchronous Publish call, which is more appropriate for the collector.

cc @ibawt

At Shopify do you push the complete OTLP Request over PubSub or are you unraffling the content. Let me elaborate: For spans, the ExportTraceServiceRequest contains a repeated ResourceSpans. The request is already a mini-batch. I'm going to propose to put the ResourceSpans on Pubsub (or any message bus).

We publish a full ExportTraceServiceRequest. We treat PubSub as a transport for OTLP, which makes things easier for downstream consumers. In particular, we can reuse code between the collector and the consumer. For example, we have an exporter from PubSub to GCS (Avro) that we'll eventually move into the collector, so it helps if that receives the data it would receive if it were in the collector instead.

Specifically, for traces pdata.Traces, we Publish the gzipped bytes returned from traces.ToOtlpProtoBytes().

While it's a big effort, I do wish that OTLP had a FullSpan or similar type which was a normalized span with resource / instrumentation library built in. Having the ability to pack a self-contained span can help with cases like message queues, downstream processing like metrics updating, or even updating a trace with a new span, generally model better with span-per-message when dealing with retries and such.

github-actions · 2021-01-22T06:03:06Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

alexvanboxel · 2021-01-22T11:47:07Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Commenting... as this is a long-running branch. First OTEP needs to be written.

Allows export and reception of OTLP data over Google Pubsub. With the ability to set subscriptions and topics for each of the components.

github-actions · 2021-02-17T05:26:46Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-02-24T05:27:18Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

alexvanboxel requested a review from a team December 22, 2020 20:25

github-actions bot assigned anuraaga Dec 22, 2020

alexvanboxel force-pushed the feature/pubsub branch 2 times, most recently from a1c97ed to fb808a5 Compare December 23, 2020 11:09

anuraaga reviewed Dec 24, 2020

View reviewed changes

alexvanboxel marked this pull request as draft December 24, 2020 06:55

alexvanboxel force-pushed the feature/pubsub branch from fb808a5 to bc8c7fd Compare December 25, 2020 09:37

fbogsany reviewed Jan 6, 2021

View reviewed changes

github-actions bot added the Stale label Jan 22, 2021

anuraaga removed the Stale label Jan 22, 2021

Base automatically changed from master to main January 28, 2021 00:57

Add Google Pubsub as an OTLP exporter/receiver

85ce288

Allows export and reception of OTLP data over Google Pubsub. With the ability to set subscriptions and topics for each of the components.

alexvanboxel force-pushed the feature/pubsub branch from bc8c7fd to 85ce288 Compare February 3, 2021 18:03

Changed Receiver to Low level API

aa9bd9e

gramidt mentioned this pull request Feb 12, 2021

REQUEST: New membership for @gramidt open-telemetry/community#648

Closed

6 tasks

github-actions bot added the Stale label Feb 17, 2021

github-actions bot closed this Feb 24, 2021

This was referenced May 26, 2021

Add Google Pubsub exporter for OTLP messages - PR1 #3551

Merged

Add Google Pubsub receiver for OTLP messages - PR1 #3552

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Google Pubsub as an OTLP exporter/receiver #1892

Add Google Pubsub as an OTLP exporter/receiver #1892

alexvanboxel commented Dec 22, 2020

codecov bot commented Dec 22, 2020 •

edited

Loading

alexvanboxel commented Dec 23, 2020

anuraaga left a comment

anuraaga Dec 24, 2020

anuraaga Dec 24, 2020

gramidt Dec 30, 2020 •

edited

Loading

anuraaga Dec 24, 2020

alexvanboxel Dec 24, 2020

gramidt Dec 30, 2020

anuraaga Dec 24, 2020

fbogsany Jan 6, 2021

alexvanboxel Jan 12, 2021

alexvanboxel Jan 12, 2021

fbogsany Jan 13, 2021

anuraaga Jan 15, 2021

github-actions bot commented Jan 22, 2021

alexvanboxel commented Jan 22, 2021 •

edited

Loading

github-actions bot commented Feb 17, 2021

github-actions bot commented Feb 24, 2021


		This exporter sends OTLP messages to a Google Cloud [Pubsub](https://cloud.google.com/pubsub) topic.

		The following configuration options are supported:

Add Google Pubsub as an OTLP exporter/receiver #1892

Add Google Pubsub as an OTLP exporter/receiver #1892

Conversation

alexvanboxel commented Dec 22, 2020

codecov bot commented Dec 22, 2020 • edited Loading

Codecov Report

alexvanboxel commented Dec 23, 2020

anuraaga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gramidt Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 22, 2021

alexvanboxel commented Jan 22, 2021 • edited Loading

github-actions bot commented Feb 17, 2021

github-actions bot commented Feb 24, 2021

codecov bot commented Dec 22, 2020 •

edited

Loading

gramidt Dec 30, 2020 •

edited

Loading

alexvanboxel commented Jan 22, 2021 •

edited

Loading