Metrics/tracing dependency abstraction and multi-module repository (#653

)

Makefile

@@ -10,20 +10,18 @@ TEST_TIMEOUT := 3m
 TEST_ARG ?= -race -v -timeout $(TEST_TIMEOUT)
 
 INTEG_TEST_ROOT := ./test
-COVER_ROOT := $(BUILD)/coverage
+COVER_ROOT := $(abspath $(BUILD)/coverage)
 UT_COVER_FILE := $(COVER_ROOT)/unit_test_cover.out
 INTEG_ZERO_CACHE_COVER_FILE := $(COVER_ROOT)/integ_test_zero_cache_cover.out
 INTEG_NORMAL_CACHE_COVER_FILE := $(COVER_ROOT)/integ_test_normal_cache_cover.out
 
 # Automatically gather all srcs
 ALL_SRC :=  $(shell find . -name "*.go")
 
+MOD_DIRS := $(sort $(dir $(shell find . -name go.mod)))
 UT_DIRS := $(filter-out $(INTEG_TEST_ROOT)%, $(sort $(dir $(filter %_test.go,$(ALL_SRC)))))
 INTEG_TEST_DIRS := $(sort $(dir $(shell find $(INTEG_TEST_ROOT) -name *_test.go)))
 
-# Files that needs to run lint. Excludes testify mocks.
-LINT_SRC := $(filter-out ./mocks/%,$(ALL_SRC))
-
 # `make copyright` or depend on "copyright" to force-run licensegen,
 # or depend on $(BUILD)/copyright to let it run as needed.
 copyright $(BUILD)/copyright:
@@ -41,20 +39,20 @@ unit-test: $(BUILD)/dummy
 	@echo "mode: atomic" > $(UT_COVER_FILE)
 	@for dir in $(UT_DIRS); do \
 		mkdir -p $(COVER_ROOT)/"$$dir"; \
-		go test "$$dir" $(TEST_ARG) -coverprofile=$(COVER_ROOT)/"$$dir"/cover.out || exit 1; \
+		(cd "$$dir" && go test . $(TEST_ARG) -coverprofile=$(COVER_ROOT)/"$$dir"/cover.out) || exit 1; \
 		cat $(COVER_ROOT)/"$$dir"/cover.out | grep -v "mode: atomic" >> $(UT_COVER_FILE); \
 	done;
 
 integration-test-zero-cache: $(BUILD)/dummy
 	@mkdir -p $(COVER_ROOT)
 	@for dir in $(INTEG_TEST_DIRS); do \
-		WORKFLOW_CACHE_SIZE=0 go test $(TEST_ARG) "$$dir" -coverprofile=$(INTEG_ZERO_CACHE_COVER_FILE) -coverpkg=./... || exit 1; \
+		(cd "$$dir" &&WORKFLOW_CACHE_SIZE=0 go test $(TEST_ARG) . -coverprofile=$(INTEG_ZERO_CACHE_COVER_FILE) -coverpkg=./...) || exit 1; \
 	done;
 
 integration-test-normal-cache: $(BUILD)/dummy
 	@mkdir -p $(COVER_ROOT)
 	@for dir in $(INTEG_TEST_DIRS); do \
-		go test $(TEST_ARG) "$$dir" -coverprofile=$(INTEG_NORMAL_CACHE_COVER_FILE) -coverpkg=./... || exit 1; \
+		(cd "$$dir" && go test $(TEST_ARG) . -coverprofile=$(INTEG_NORMAL_CACHE_COVER_FILE) -coverpkg=./...) || exit 1; \
 	done;
 
 test: unit-test integration-test-zero-cache integration-test-normal-cache
@@ -71,55 +69,31 @@ cover: $(COVER_ROOT)/cover.out
 cover_ci: $(COVER_ROOT)/cover.out
 	goveralls -coverprofile=$(COVER_ROOT)/cover.out -service=buildkite || echo -e "\x1b[31mCoveralls failed\x1b[m";
 
-# golint fails to report many lint failures if it is only given a single file
-# to work on at a time, and it can't handle multiple packages at once, *and*
-# we can't exclude files from its checks, so for best results we need to give
-# it a whitelist of every file in every package that we want linted, per package.
-#
-# so lint + this golint func works like:
-# - iterate over all lintable dirs (outputs "./folder/")
-# - find .go files in a dir (via wildcard, so not recursively)
-# - filter to only files in LINT_SRC
-# - if it's not empty, run golint against the list
-define lint_if_present
-test -n "$1" && golint -set_exit_status $1
-endef
-
-lint: $(ALL_SRC)
-	GO111MODULE=off go get -u golang.org/x/lint/golint
-	@$(foreach pkg,\
-		$(sort $(dir $(LINT_SRC))), \
-		$(call lint_if_present,$(filter $(wildcard $(pkg)*.go),$(LINT_SRC))) || ERR=1; \
-	) test -z "$$ERR" || exit 1
-	@OUTPUT=`gofmt -l $(ALL_SRC) 2>&1`; \
-	if [ "$$OUTPUT" ]; then \
-		echo "Run 'make fmt'. gofmt must be run on the following files:"; \
-		echo "$$OUTPUT"; \
-		exit 1; \
-	fi
-
 vet: $(ALL_SRC)
-	go vet ./...
+	@for dir in $(MOD_DIRS); do \
+		(cd "$$dir" && go vet ./...) || exit 1; \
+	done;
 
 staticcheck: $(ALL_SRC)
 	go install honnef.co/go/tools/cmd/staticcheck@latest
-	staticcheck ./...
+	@for dir in $(MOD_DIRS); do \
+		(cd "$$dir" && staticcheck ./...) || exit 1; \
+	done;
 
 errcheck: $(ALL_SRC)
 	GO111MODULE=off go get -u github.com/kisielk/errcheck
-	errcheck ./...
+	@for dir in $(MOD_DIRS); do \
+		(cd "$$dir" && errcheck ./...) || exit 1; \
+	done;
 
 fmt:
 	@gofmt -w $(ALL_SRC)
 
 clean:
 	rm -rf $(BUILD)
 
-# golint is intentionally not included in the standard check since it is
-# deprecated and inflexible, but it remains available as a utility
 check: vet errcheck staticcheck copyright bins
 
-
 ##### Fossa #####
 fossa-install: 
 	curl -H 'Cache-Control: no-cache' https://raw.githubusercontent.com/fossas/fossa-cli/master/install.sh | bash

activity/activity.go

@@ -27,9 +27,8 @@ package activity
 import (
 	"context"
 
-	"github.com/uber-go/tally/v4"
-
 	"go.temporal.io/sdk/internal"
+	"go.temporal.io/sdk/internal/common/metrics"
 	"go.temporal.io/sdk/log"
 )
 
@@ -61,9 +60,9 @@ func GetLogger(ctx context.Context) log.Logger {
 	return internal.GetActivityLogger(ctx)
 }
 
-// GetMetricsScope returns a metrics scope that can be used in activity
-func GetMetricsScope(ctx context.Context) tally.Scope {
-	return internal.GetActivityMetricsScope(ctx)
+// GetMetricsHandler returns a metrics handler that can be used in activity
+func GetMetricsHandler(ctx context.Context) metrics.Handler {
+	return internal.GetActivityMetricsHandler(ctx)
 }
 
 // RecordHeartbeat sends heartbeat for the currently executing activity

client/client.go

@@ -37,6 +37,7 @@ import (
 
 	"go.temporal.io/sdk/converter"
 	"go.temporal.io/sdk/internal"
+	"go.temporal.io/sdk/internal/common/metrics"
 )
 
 const (
@@ -408,6 +409,28 @@ type (
 	}
 )
 
+// MetricsHandler is a handler for metrics emitted by the SDK. This interface is
+// intentionally limited to only what the SDK needs to emit metrics and is not
+// built to be a general purpose metrics abstraction for all uses.
+//
+// A common implementation is at
+// go.temporal.io/sdk/contrib/tally.NewMetricsHandler. The MetricsNopHandler is
+// a noop handler. A handler may implement "Unwrap() client.MetricsHandler" if
+// it wraps a handler.
+type MetricsHandler = metrics.Handler
+
+// MetricsCounter is an ever-increasing counter.
+type MetricsCounter = metrics.Counter
+
+// MetricsGauge can be set to any float.
+type MetricsGauge = metrics.Gauge
+
+// MetricsTimer records time durations.
+type MetricsTimer = metrics.Timer
+
+// MetricsNopHandler is a noop handler that does nothing with the metrics.
+var MetricsNopHandler = metrics.NopHandler
+
 // NewClient creates an instance of a workflow client
 func NewClient(options Options) (Client, error) {
 	return internal.NewClient(options)

contrib/opentelemetry/go.mod

@@ -0,0 +1,13 @@
+module go.temporal.io/sdk/contrib/opentelemetry
+
+go 1.16
+
+require (
+	github.com/stretchr/testify v1.7.0
+	go.opentelemetry.io/otel v1.2.0
+	go.opentelemetry.io/otel/sdk v1.2.0
+	go.opentelemetry.io/otel/trace v1.2.0
+	go.temporal.io/sdk v1.11.1
+)
+
+replace go.temporal.io/sdk => ../../