Add Google Pubsub as an OTLP exporter/receiver #7172

alexvanboxel · 2022-01-14T07:49:27Z

Description:
Add the implementation for Google Pubsub exporter and receiver for OTLP messages. The export is able to send OTEP messages to a Google Pubsub topic. The receiver and exporter are pushed in one PR as they work in pairs. A typical scenario is having an ingestion cluster (gRPC) pushing messages to Pubsub and then having receivers read again from Pubsub, having Pubsub as a buffer. That's why it doesn't make sense to have them reviewed separately.

This PR replaces the following PR:
#3879

The implementation has drifted and also includes several enhancements, it doesn't make sense to put it over the other PR.

The implementation is currently in use on production at Collibra.

Link to tracking Issue:
#1802

Testing:
exporter 93.1% test coverage
receiver 83.4% test coverage

Documentation:
Up-to-date README

bogdandrutu · 2022-01-20T17:35:37Z

@dashpole do you want to be the sponsor for these components? If yes, please review :)

dashpole · 2022-01-20T20:32:38Z

Yes, I'll gladly sponsor the component

CHANGELOG.md

exporter/googlecloudpubsubexporter/README.md

exporter/googlecloudpubsubexporter/config.go

receiver/googlecloudpubsubreceiver/internal/handler.go

receiver/googlecloudpubsubreceiver/receiver.go

alexvanboxel · 2022-01-21T11:03:17Z

Thanks @dashpole for the review, I'm on it.

MovieStoreGuy · 2022-01-25T02:57:53Z

exporter/googlecloudpubsubexporter/config_test.go

@@ -29,16 +28,15 @@ import (

 func TestLoadConfig(t *testing.T) {
 factories, err := componenttest.NopFactories()
- assert.Nil(t, err)
+ assert.NoError(t, err)


This should be require since it shouldn't allow the test to progress if this fails.

MovieStoreGuy · 2022-01-25T03:05:58Z

exporter/googlecloudpubsubexporter/exporter.go

+ if ex.userAgent != "" {
+ dialOpts = append(dialOpts, grpc.WithUserAgent(ex.userAgent))
+ }
+ conn, _ := grpc.Dial(ex.config.Endpoint, append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))...)


Is this error able to be captured elsewhere?

Just worried it could have a dead conn or partially set connection and it "working" without actually doing work.

Is this error able to be captured elsewhere?

I think this is the only thing I didn't fix because this is only for testing/mocking.

If it is only for testing/mocking, it probably shouldn't be part of the user-facing config (config can have unexported structs without mapstructure tags, IIRC). Otherwise, we should consider it part of the user-facing config, and should handle errors.

alexvanboxel · 2022-02-06T20:16:59Z

@dashpole I think I covered most of your concerns now. I did rebase the PR on master (to fix the errors in the changelog. I put 2 commits on top for the receiver and exporter respectively.

dashpole · 2022-02-07T15:16:06Z

CHANGELOG.md

-## 🚀 New components 🚀
+## 💡 Enhancements 💡
+
+- `googlecloudpubsubreceiver` Added implementation of Google Cloud Pubsub receiver. (#1802)


nit: duplicated line

dashpole · 2022-02-07T15:24:53Z

exporter/googlecloudpubsubexporter/README.md

+ topic: otlp-traces
+ watermark: 
+ behavior: earliest
+ allow_drift: 1h


Should we default allow_drift to 1h? IIUC, it doesn't apply if behavior == current, and if someone sets it to earliest, it seems nice to have the recommended value be the default to simplify config.

If I would set another default I would set it to MAX duration then? That would make more sense.

What is MAX duration? I'm not particularly concerned with what default you choose (you are the expert). Whatever you choose, it should work for most users.

dashpole · 2022-02-07T15:29:45Z

exporter/googlecloudpubsubexporter/exporter.go

+
+const (
+ OtlpProtoTrace encoding = iota
+ OtlpProtoMetric = iota


nit: Is this still need to be exported?

Can you also unexport constants below as well?

dashpole · 2022-02-07T15:33:44Z

exporter/googlecloudpubsubexporter/exporter.go

+ if ex.userAgent != "" {
+ dialOpts = append(dialOpts, grpc.WithUserAgent(ex.userAgent))
+ }
+ conn, _ := grpc.Dial(ex.config.Endpoint, append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))...)


If it is only for testing/mocking, it probably shouldn't be part of the user-facing config (config can have unexported structs without mapstructure tags, IIRC). Otherwise, we should consider it part of the user-facing config, and should handle errors.

github-actions · 2022-02-22T05:15:26Z

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

fbogsany · 2022-02-25T14:53:59Z

exporter/googlecloudpubsubexporter/README.md

@@ -9,6 +9,12 @@ The following configuration options are supported:
 * `project` (Optional): The Google Cloud Project of the topics.
 * `topic` (Required): The topic name to receive OTLP data over. The topic name should be a fully qualified resource
 name (eg: `projects/otel-project/topics/otlp`).
+* `compression` (Optional): Set the payload compression, only `gzip` is supported. Default is no compression.
+* `watermark` Behaviour of how the `ce-time` attribute is set (see watermark section for more info)
+ * `behavior` (Optional): `current` sets the `ce-time` attribute to the system clock, `earlierst` sets the attribute to 


Suggested change

* `behavior` (Optional): `current` sets the `ce-time` attribute to the system clock, `earlierst` sets the attribute to

* `behavior` (Optional): `current` sets the `ce-time` attribute to the system clock, `earliest` sets the attribute to

fbogsany · 2022-02-25T14:59:49Z

receiver/googlecloudpubsubreceiver/config.go

@@ -42,6 +42,8 @@ type Config struct {
 Subscription string `mapstructure:"subscription"`
 // Lock down the encoding of the payload, leave empty for attribute based detection
 Encoding string `mapstructure:"encoding"`
+ // Lock down the encoding of the payload, leave empty for attribute based detection


Suggested change

// Lock down the encoding of the payload, leave empty for attribute based detection

// Lock down the compression of the payload, leave empty for attribute based detection

github-actions · 2022-03-12T05:15:25Z

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

alexvanboxel · 2022-03-13T08:11:23Z

Sorry, to have let this slack. I have a re-opened a mergeable PR with all comments (also the suggested edits here: #8391)

…en-telemetry#7172) * Remove Get prefix from consumer package functions * Provide deprecation path * Use generics to provide common implementation for structs * Update deprecation version * Experiment with `Data` accessor --------- Co-authored-by: Evan Bradley <[email protected]>

alexvanboxel requested a review from a team as a code owner January 14, 2022 07:49

alexvanboxel requested a review from codeboten January 14, 2022 07:49

github-actions bot assigned bogdandrutu Jan 14, 2022

alexvanboxel force-pushed the feature/pubsub-full branch 15 times, most recently from 939aea2 to 38421b2 Compare January 20, 2022 06:30

dashpole assigned dashpole and unassigned bogdandrutu Jan 20, 2022

dashpole reviewed Jan 20, 2022

View reviewed changes

MovieStoreGuy reviewed Jan 25, 2022

View reviewed changes

alexvanboxel added 3 commits February 6, 2022 14:18

Add Google Pubsub as an OTLP exporter/receiver

da25a0e

Fix Pull Request comment on the GoogleCloudPububExporter

da06613

Fix Pull Request comment on the GoogleCloudPububReceiver

da1c247

alexvanboxel force-pushed the feature/pubsub-full branch from 38421b2 to da1c247 Compare February 6, 2022 13:20

alexvanboxel force-pushed the feature/pubsub-full branch from 0d327f5 to ffa09a3 Compare February 6, 2022 17:15

GoogleCloudPubsub linting

d578677

alexvanboxel force-pushed the feature/pubsub-full branch from ffa09a3 to d578677 Compare February 6, 2022 17:59

alexvanboxel requested a review from dashpole February 6, 2022 20:14

dashpole reviewed Feb 7, 2022

View reviewed changes

github-actions bot added the Stale label Feb 22, 2022

fbogsany reviewed Feb 25, 2022

View reviewed changes

github-actions bot closed this Mar 12, 2022

alexvanboxel mentioned this pull request Mar 12, 2022

[exporter | receiver/googlecloudpubsub] Initial implementation #8391

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Google Pubsub as an OTLP exporter/receiver #7172

Add Google Pubsub as an OTLP exporter/receiver #7172

alexvanboxel commented Jan 14, 2022

bogdandrutu commented Jan 20, 2022

dashpole commented Jan 20, 2022

alexvanboxel commented Jan 21, 2022

MovieStoreGuy Jan 25, 2022

MovieStoreGuy Jan 25, 2022

alexvanboxel Feb 6, 2022

dashpole Feb 7, 2022

alexvanboxel commented Feb 6, 2022

dashpole Feb 7, 2022

dashpole Feb 7, 2022

alexvanboxel Feb 7, 2022

dashpole Feb 7, 2022

dashpole Feb 7, 2022

dashpole Feb 7, 2022

dashpole Feb 7, 2022

github-actions bot commented Feb 22, 2022

fbogsany Feb 25, 2022

fbogsany Feb 25, 2022

github-actions bot commented Mar 12, 2022

alexvanboxel commented Mar 13, 2022

	* `behavior` (Optional): `current` sets the `ce-time` attribute to the system clock, `earlierst` sets the attribute to
	* `behavior` (Optional): `current` sets the `ce-time` attribute to the system clock, `earliest` sets the attribute to

	// Lock down the encoding of the payload, leave empty for attribute based detection
	// Lock down the compression of the payload, leave empty for attribute based detection

Add Google Pubsub as an OTLP exporter/receiver #7172

Add Google Pubsub as an OTLP exporter/receiver #7172

Conversation

alexvanboxel commented Jan 14, 2022

bogdandrutu commented Jan 20, 2022

dashpole commented Jan 20, 2022

alexvanboxel commented Jan 21, 2022

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

alexvanboxel commented Feb 6, 2022

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 Feb 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 12, 2022

alexvanboxel commented Mar 13, 2022