Interceptor Redesign/Rewrite and OpenTelemetry Implementation #610

cretz · 2021-11-01T15:57:32Z

What was changed

See the proposal at temporalio/proposals#45.

All existing interceptor code was remove (in fact, entire interceptors package was removed)
New interceptor package added with support for client, workflow, and activity interception on par with other SDKs
OpenTelemetry support on top of new interceptors
OpenTracing support on top of new interceptors
Various tests and utilities for supporting new interceptors

The review looks larger than it is due to Go boilerplate.

Why?

Go's existing interceptor design was limited and didn't support tracing, activities, clients, etc.

Checklist

Closes Add ActivityInterceptor #265
Closes Add open tracing support #526
Closes Redesign interceptors to match the Java / Node.js capabilities #529
Closes Support OpenTelemetry tracing #602
There is a spot or two in the review that I'll point out that may warrant discussion
Document Go SDK Interceptors documentation#714 will track public interceptor documentation
Interceptor Samples samples-go#150

# Conflicts: # test/integration_test.go # test/workflow_test.go

internal/client.go

interceptor/interceptor.go

internal/internal_activity.go

internal/internal_workflow.go

internal/internal_workflow_client.go

interceptor/interceptor.go

interceptor/tracing_interceptor.go

internal/internal_worker.go

interceptor/tracing_interceptor.go

bergundy

Approving even though I did not read this through.

Sushisource

🚄 Awesome! This cleans up a lot of stuff, and users are going to appreciate the flexibility.

interceptor/tracing_interceptor.go

internal/internal_workflow.go

opentelemetry/tracing_interceptor.go

meiliang86 · 2021-11-09T18:14:37Z

@cretz Thanks for working with us! I'll take a look at this PR and follow up with you in case there's any further change needed.

Makefile

interceptor/tracing_interceptor.go

internal/internal_workflow.go

internal/internal_workflow_client.go

internal/workflow.go

meiliang86

Thanks for cleaning this up, Chad!

internal/interceptor_header.go

internal/internal_activity.go

internal/internal_workflow_client.go

internal/workflow_test.go

opentelemetry/tracing_interceptor.go

opentracing/interceptor.go

# Conflicts: # internal/internal_task_pollers.go

cretz · 2021-11-11T16:46:17Z

Marking outstanding conversations sans response since yesterday as "resolved"

# Conflicts: # test/integration_test.go # test/workflow_test.go

cretz added 9 commits October 20, 2021 11:07

Interceptor work

68dcaa3

Work on activity interceptors

3e400df

Work on client interceptors

a2ca3bc

OpenTracing implementation using interceptors

373dc98

Abstract tracing interceptors

8dc766e

Add OpenTelemetry tracing interceptor

b4a883d

Work on interceptor testing and docs

76b43be

Work on godoc and tests

2276712

Merge remote-tracking branch 'remotes/origin/master' into interceptors

9f4c463

# Conflicts: # test/integration_test.go # test/workflow_test.go

cretz requested review from alexshtin, bergundy, mfateev, Spikhalskiy, Sushisource and vitarb as code owners November 1, 2021 15:57

cretz commented Nov 1, 2021

View reviewed changes

internal/client.go Show resolved Hide resolved

interceptor/interceptor.go Show resolved Hide resolved

internal/internal_activity.go Outdated Show resolved Hide resolved

internal/internal_workflow.go Show resolved Hide resolved

internal/internal_workflow_client.go Show resolved Hide resolved