Migrate updates to Prometheus receiver from opencensus-service

DESIGN.md

@@ -90,7 +90,7 @@ running. By default, the Agent is available on port 55678.
    there is no identifier in the first message, Agent should drop the whole
    message and return an error to the client. In addition, the first message
    MAY contain additional data (such as `Span`s). As long as it has a valid
-   identifier assoicated, Agent should handle the data properly, as if they
+   identifier associated, Agent should handle the data properly, as if they
    were sent in a subsequent message. Identifier is no longer needed once the
    streams are established.
 3. On Library side, if connection to Agent failed, Library should retry
@@ -139,7 +139,7 @@ library, and `Node` is not required for the subsequent messages from libraries.
 
 #### <a name="agent-impl-core"></a>Agent Core
 
-Most functionalities of Agent are in Agent Core. Agent Core's responsibilies
+Most functionalities of Agent are in Agent Core. Agent Core's responsibilities
 include:
 
 1. Accept `SpanProto` from each receiver. Note that the `SpanProto`s that are

Makefile

@@ -4,17 +4,26 @@ ALL_SRC := $(shell find . -name '*.go' \
                                 -not -path './testbed/*' \
                                 -type f | sort)
 
+# All source code and documents. Used in spell check.
+ALL_SRC_AND_DOC := $(shell find . \( -name "*.md" -o -name "*.go" -o -name "*.yaml" \) \
+                                -not -path './vendor/*' \
+                                -type f | sort)
+
 # ALL_PKGS is used with 'go cover'
 ALL_PKGS := $(shell go list $(sort $(dir $(ALL_SRC))))
 
 GOTEST_OPT?= -race -timeout 30s
 GOTEST_OPT_WITH_COVERAGE = $(GOTEST_OPT) -coverprofile=coverage.txt -covermode=atomic
 GOTEST=go test
 GOFMT=gofmt
+GOIMPORTS=goimports
 GOLINT=golint
 GOVET=go vet
 GOOS=$(shell go env GOOS)
 ADDLICENCESE= addlicense
+MISSPELL=misspell -error
+MISSPELL_CORRECTION=misspell -w
+STATICCHECK=staticcheck
 
 GIT_SHA=$(shell git rev-parse --short HEAD)
 BUILD_INFO_IMPORT_PATH=github.com/open-telemetry/opentelemetry-service/internal/version
@@ -30,10 +39,10 @@ all-pkgs:
 all-srcs:
 	@echo $(ALL_SRC) | tr ' ' '\n' | sort
 
-.DEFAULT_GOAL := addlicense-fmt-vet-lint-test
+.DEFAULT_GOAL := addlicense-fmt-vet-lint-goimports-misspell-staticcheck-test
 
-.PHONY: addlicense-fmt-vet-lint-test
-addlicense-fmt-vet-lint-test: addlicense fmt vet lint test
+.PHONY: addlicense-fmt-vet-lint-goimports-misspell-staticcheck-test
+addlicense-fmt-vet-lint-goimports-misspell-staticcheck-test: addlicense fmt vet lint goimports misspell staticcheck test
 
 .PHONY: e2e-test
 e2e-test: otelsvc
@@ -44,7 +53,7 @@ test:
 	$(GOTEST) $(GOTEST_OPT) $(ALL_PKGS)
 
 .PHONY: travis-ci
-travis-ci: fmt vet lint test-with-cover otelsvc
+travis-ci: fmt vet lint goimports misspell staticcheck test-with-cover otelsvc
 	$(MAKE) -C testbed install-tools
 	$(MAKE) -C testbed runtests
 
@@ -90,15 +99,42 @@ lint:
 	    echo "Lint finished successfully"; \
 	fi
 
+.PHONY: goimports
+goimports:
+	@IMPORTSOUT=`$(GOIMPORTS) -d . 2>&1`; \
+	if [ "$$IMPORTSOUT" ]; then \
+		echo "$(GOIMPORTS) FAILED => fix the following goimports errors:\n"; \
+		echo "$$IMPORTSOUT\n"; \
+		exit 1; \
+	else \
+	    echo "Goimports finished successfully"; \
+	fi
+
+.PHONY: misspell
+misspell:
+	$(MISSPELL) $(ALL_SRC_AND_DOC)
+
+.PHONY: misspell-correction
+misspell-correction:
+	$(MISSPELL_CORRECTION) $(ALL_SRC_AND_DOC)
+
+.PHONY: staticcheck
+staticcheck:
+	$(STATICCHECK) ./...
+
 .PHONY: vet
 vet:
 	@$(GOVET) ./...
 	@echo "Vet finished successfully"
 
 .PHONY: install-tools
 install-tools:
-	GO111MODULE=on go install github.com/google/addlicense
-	GO111MODULE=on go install golang.org/x/lint/golint
+	GO111MODULE=on go install \
+	  github.com/google/addlicense \
+	  golang.org/x/lint/golint \
+	  golang.org/x/tools/cmd/goimports \
+	  github.com/client9/misspell/cmd/misspell \
+	  honnef.co/go/tools/cmd/staticcheck
 
 .PHONY: otelsvc
 otelsvc:

README.md

@@ -55,6 +55,8 @@ deployment methods:
 
 For the detailed design specs, please see [DESIGN.md](DESIGN.md).
 
+For the future vision of OpenTelemetry Service please see [vision.md](docs/vision.md).
+
 ## <a name="deploy"></a>Deployment
 
 The OpenTelemetry Service can be deployed in a variety of different ways
@@ -89,23 +91,22 @@ Create an Agent [configuration](#config) file based on the options described
 below. By default, the Agent has the `opencensus` receiver enabled, but no
 exporters configured.
 
-Build the Agent, see [Usage](##agent-usage),
-and start it:
+Build the Agent and start it with the example configuration:
 
 ```shell
-$ ./bin/$(go env GOOS)/otelsvc
+$ ./bin/$(go env GOOS)/otelsvc  --config ./examples/demotrace/otel-agent-config.yaml
 $ 2018/10/08 21:38:00 Running OpenTelemetry receiver as a gRPC service at "127.0.0.1:55678"
 ```
 
 Create an Collector [configuration](#config) file based on the options
 described below. By default, the Collector has the `opencensus` receiver
 enabled, but no exporters configured.
 
-Build the Collector and start it:
+Build the Collector and start it with the example configuration:
 
 ```shell
 $ make otelsvc
-$ ./bin/$($GOOS)/otelsvc
+$ ./bin/$($GOOS)/otelsvc --config ./examples/demotrace/otel-collector-config.yaml
 ```
 
 Run the demo application:
@@ -121,14 +122,29 @@ 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 enabled exporter
+needs to be configured.
+
+*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).
+
+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
@@ -181,6 +197,33 @@ 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 in the configuration to be
+included in a pipeline and each receiver/processor/exporter can be used in more
+than one pipeline.
+
+*Note:* For processor(s) referenced in multiple pipelines, each pipeline will
+get a separate instance of that processor(s). This is in contrast to
+receiver(s)/exporter(s) referenced in multiple pipelines, one instance of
+a receiver/exporter is reference by all the pipelines.
+
+The following is an example pipeline configuration. For more information, refer
+to [pipeline documentation](docs/pipelines.md)
+```yaml
+pipelines:
+  traces:
+    receivers: [examplereceiver]
+    processors: [exampleprocessor]
+    exporters: [exampleexporter]
+
+```
+
 ### <a name="config-diagnostics"></a>Diagnostics
 
 zPages is provided for monitoring running by default on port ``55679``.
@@ -207,7 +250,10 @@ 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
@@ -300,7 +346,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).

internal/compression/grpc/grpc.go → compression/grpc/grpc.go

@@ -19,7 +19,7 @@ import (
 
 	"google.golang.org/grpc/encoding/gzip"
 
-	"github.com/open-telemetry/opentelemetry-service/internal/compression"
+	"github.com/open-telemetry/opentelemetry-service/compression"
 )
 
 var (

internal/compression/grpc/grpc_test.go → compression/grpc/grpc_test.go

@@ -17,7 +17,7 @@ package grpc
 import (
 	"testing"
 
-	"github.com/open-telemetry/opentelemetry-service/internal/compression"
+	"github.com/open-telemetry/opentelemetry-service/compression"
 )
 
 func TestGetGRPCCompressionKey(t *testing.T) {

config/config.go

@@ -55,6 +55,8 @@ const (
 	errPipelineExporterNotExists
 	errMetricPipelineCannotHaveProcessors
 	errUnmarshalError
+	errMissingReceivers
+	// TODO(ccaraman): Add an error for missing Processors with corresponding test cases.
 )
 
 type configError struct {
@@ -175,8 +177,21 @@ 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
+	// 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)
+	receivers := make(configmodels.Receivers)
 
 	// Iterate over input map and create a config for each.
 	for key := range keyMap {
@@ -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)
 		}
 
@@ -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
 }
 
@@ -242,7 +271,7 @@ func loadExporters(v *viper.Viper, factories map[string]exporter.Factory) (confi
 	keyMap := v.GetStringMap(exportersKeyName)
 
 	// Prepare resulting map
-	exporters := make(configmodels.Exporters, 0)
+	exporters := make(configmodels.Exporters)
 
 	// Iterate over exporters and create a config for each.
 	for key := range keyMap {
@@ -299,7 +328,7 @@ func loadProcessors(v *viper.Viper, factories map[string]processor.Factory) (con
 	keyMap := v.GetStringMap(processorsKeyName)
 
 	// Prepare resulting map.
-	processors := make(configmodels.Processors, 0)
+	processors := make(configmodels.Processors)
 
 	// Iterate over processors and create a config for each.
 	for key := range keyMap {
@@ -356,7 +385,7 @@ func loadPipelines(v *viper.Viper) (configmodels.Pipelines, error) {
 	keyMap := v.GetStringMap(pipelinesKeyName)
 
 	// Prepare resulting map.
-	pipelines := make(configmodels.Pipelines, 0)
+	pipelines := make(configmodels.Pipelines)
 
 	// Iterate over input map and create a config for each.
 	for key := range keyMap {
@@ -496,6 +525,7 @@ func validatePipelineReceivers(
 				zap.String("receiver", ref))
 		}
 	}
+
 	pipeline.Receivers = rs
 
 	return nil
@@ -594,6 +624,9 @@ func validatePipelineProcessors(
 	return nil
 }
 
+// TODO(ccaraman): Determine if there a way to consolidate the validate receivers apart of the loadReceivers method.
+//	 Currently, validateReceivers needs to be invoked after validatePipelineReceivers because that bit of
+//	 code checks if a receiver is enabled prior to finalizing the pipelines.
 func validateReceivers(cfg *configmodels.Config) {
 	// Remove disabled receivers.
 	for name, rcv := range cfg.Receivers {

config/config_test.go

@@ -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},
+		{name: "missing-receivers", expected: errMissingReceivers},
 		{name: "missing-exporters"},
 		{name: "missing-processors"},
 		{name: "invalid-receiver-name"},
@@ -215,7 +216,7 @@ func TestDecodeConfig_Invalid(t *testing.T) {
 			t, path.Join(".", "testdata", test.name+".yaml"), receivers, processors, exporters,
 		)
 		if err == nil {
-			t.Errorf("expected error but succedded on invalid config case: %s", test.name)
+			t.Errorf("expected error but succeeded on invalid config case: %s", test.name)
 		} else if test.expected != 0 {
 			cfgErr, ok := err.(*configError)
 			if !ok {