Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/master'
Browse files Browse the repository at this point in the history
Signed-off-by: Deepak <[email protected]>
  • Loading branch information
goku321 committed Oct 6, 2020
2 parents 6ceca81 + 873397e commit 91d0746
Show file tree
Hide file tree
Showing 32 changed files with 346 additions and 70 deletions.
23 changes: 18 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ We gratefully welcome improvements to documentation as well as to code.
## Getting Started

### Pre-requisites
* Install [Go](https://golang.org/doc/install) and setup GOPATH and add $GOPATH/bin in PATH
* Install [Go](https://golang.org/doc/install) and setup GOPATH and add $GOPATH/bin in PATH

This library uses Go modules to manage dependencies.

Expand Down Expand Up @@ -116,18 +116,32 @@ import (

## Testing guidelines

We strive to maintain as high code coverage as possible. Since `go test` command does not generate
We strive to maintain as high code coverage as possible. The current repository limit is set at 95%,
with some exclusions discussed below.

### Combining code coverage

We use [cover.sh](./scripts/cover.sh) script to run tests and combine code coverage from all packages
(see also [issue # 797](https://github.com/jaegertracing/jaeger/issues/797)).

### Packages with no tests

Since `go test` command does not generate
code coverage information for packages that have no test files, we have a build step (`make nocover`)
that breaks the build when such packages are discovered, with an error like this:
that breaks the build when such packages are discovered, with the following error:

```
error: at least one *_test.go file must be in all directories with go files
so that they are counted for code coverage.
If no tests are possible for a package (e.g. it only defines types), create empty_test.go
```

As the message says, all packages are required to have at least one `*_test.go` file.

### Excluding packages from testing

There are conditions that cannot be tested without external dependencies, such as a function that
creates a gocql.Session, because it requires an active connection to Cassandra database. It is
creates a `gocql.Session`, because it requires an active connection to Cassandra database. It is
recommended to isolate such functions in a separate package with bare minimum of code and add a
file `.nocover` to exclude the package from coverage calculations. The file should contain
a comment explaining why it is there, for example:
Expand All @@ -145,4 +159,3 @@ Before merging a PR make sure:

Merge the PR by using "Squash and merge" option on Github. Avoid creating merge commits.
After the merge make sure referenced issues were closed.

4 changes: 1 addition & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@


[![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/1273/badge)](https://bestpractices.coreinfrastructure.org/projects/1273)
[![Go Report Card](https://goreportcard.com/badge/github.com/jaegertracing/jaeger?style=flat-square)](https://goreportcard.com/report/github.com/jaegertracing/jaeger)
[![Mentioned in Awesome Go](https://awesome.re/mentioned-badge-flat.svg)](https://github.com/avelino/awesome-go#performance)
[![OpenTracing-1.0][ot-badge]](https://opentracing.io)

Expand Down Expand Up @@ -110,8 +109,7 @@ of routing the traffic from Zipkin libraries to the Jaeger backend.

### Deployment

* [Kubernetes templates](https://github.com/jaegertracing/jaeger-kubernetes)
* [OpenShift templates](https://github.com/jaegertracing/jaeger-openshift)
* [Jaeger Operator for Kubernetes](https://github.com/jaegertracing/jaeger-operator#getting-started)

### Components

Expand Down
7 changes: 5 additions & 2 deletions cmd/agent/app/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber/jaeger-lib/metrics"
"github.com/uber/jaeger-lib/metrics/fork"
"go.uber.org/zap"

jmetrics "github.com/jaegertracing/jaeger/pkg/metrics"
Expand Down Expand Up @@ -100,7 +101,8 @@ func withRunningAgent(t *testing.T, testcase func(string, chan error)) {
}
logger, logBuf := testutils.NewLogger()
mBldr := &jmetrics.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
mFactory, err := mBldr.CreateMetricsFactory("jaeger")
metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
mFactory := fork.New("internal", metrics.NullFactory, metricsFactory)
require.NoError(t, err)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, mFactory)
require.NoError(t, err)
Expand Down Expand Up @@ -162,7 +164,8 @@ func TestStartStopRace(t *testing.T) {
}
logger, logBuf := testutils.NewLogger()
mBldr := &jmetrics.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
mFactory, err := mBldr.CreateMetricsFactory("jaeger")
metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
mFactory := fork.New("internal", metrics.NullFactory, metricsFactory)
require.NoError(t, err)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, mFactory)
require.NoError(t, err)
Expand Down
15 changes: 15 additions & 0 deletions cmd/agent/app/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ func (b *Builder) CreateAgent(primaryProxy CollectorProxy, logger *zap.Logger, m
return nil, fmt.Errorf("cannot create processors: %w", err)
}
server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory)
b.publishOpts(mFactory)

return NewAgent(processors, server, logger), nil
}

Expand All @@ -127,6 +129,19 @@ func (b *Builder) getReporter(primaryProxy CollectorProxy) reporter.Reporter {
return reporter.NewMultiReporter(rep...)
}

func (b *Builder) publishOpts(mFactory metrics.Factory) {
internalFactory := mFactory.Namespace(metrics.NSOptions{Name: "internal"})
for _, p := range b.Processors {
prefix := fmt.Sprintf(processorPrefixFmt, p.Model, p.Protocol)
internalFactory.Gauge(metrics.Options{Name: prefix + suffixServerMaxPacketSize}).
Update(int64(p.Server.MaxPacketSize))
internalFactory.Gauge(metrics.Options{Name: prefix + suffixServerQueueSize}).
Update(int64(p.Server.QueueSize))
internalFactory.Gauge(metrics.Options{Name: prefix + suffixWorkers}).
Update(int64(p.Workers))
}
}

func (b *Builder) getProcessors(rep reporter.Reporter, mFactory metrics.Factory, logger *zap.Logger) ([]processors.Processor, error) {
retMe := make([]processors.Processor, len(b.Processors))
for idx, cfg := range b.Processors {
Expand Down
40 changes: 40 additions & 0 deletions cmd/agent/app/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber/jaeger-lib/metrics"
"github.com/uber/jaeger-lib/metrics/fork"
"github.com/uber/jaeger-lib/metrics/metricstest"
"go.uber.org/zap"
yaml "gopkg.in/yaml.v2"
Expand Down Expand Up @@ -277,3 +278,42 @@ func TestCreateCollectorProxy_UnknownReporter(t *testing.T) {
assert.Nil(t, proxy)
assert.EqualError(t, err, "unknown reporter type ")
}

func TestPublishOpts(t *testing.T) {
v := viper.New()
cfg := &Builder{}
command := cobra.Command{}
flags := &flag.FlagSet{}
AddFlags(flags)
command.PersistentFlags().AddGoFlagSet(flags)
v.BindPFlags(command.PersistentFlags())
err := command.ParseFlags([]string{
"--http-server.host-port=:8080",
"--processor.jaeger-binary.server-host-port=:1111",
"--processor.jaeger-binary.server-max-packet-size=4242",
"--processor.jaeger-binary.server-queue-size=24",
"--processor.jaeger-binary.workers=42",
})
require.NoError(t, err)
cfg.InitFromViper(v)

baseMetrics := metricstest.NewFactory(time.Second)
forkFactory := metricstest.NewFactory(time.Second)
metricsFactory := fork.New("internal", forkFactory, baseMetrics)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory)
assert.NoError(t, err)
assert.NotNil(t, agent)

forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
Name: "internal.processor.jaeger-binary.server-max-packet-size",
Value: 4242,
})
forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
Name: "internal.processor.jaeger-binary.server-queue-size",
Value: 24,
})
forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
Name: "internal.processor.jaeger-binary.workers",
Value: 42,
})
}
7 changes: 5 additions & 2 deletions cmd/agent/app/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ const (
suffixServerMaxPacketSize = "server-max-packet-size"
suffixServerSocketBufferSize = "server-socket-buffer-size"
suffixServerHostPort = "server-host-port"

processorPrefixFmt = "processor.%s-%s."

// HTTPServerHostPort is the flag for HTTP endpoint
HTTPServerHostPort = "http-server.host-port"
)
Expand All @@ -48,7 +51,7 @@ var defaultProcessors = []struct {
// AddFlags adds flags for Builder.
func AddFlags(flags *flag.FlagSet) {
for _, p := range defaultProcessors {
prefix := fmt.Sprintf("processor.%s-%s.", p.model, p.protocol)
prefix := fmt.Sprintf(processorPrefixFmt, p.model, p.protocol)
flags.Int(prefix+suffixWorkers, defaultServerWorkers, "how many workers the processor should run")
flags.Int(prefix+suffixServerQueueSize, defaultQueueSize, "length of the queue for the UDP server")
flags.Int(prefix+suffixServerMaxPacketSize, defaultMaxPacketSize, "max packet size for the UDP server")
Expand All @@ -69,7 +72,7 @@ func AddOTELFlags(flags *flag.FlagSet) {
// InitFromViper initializes Builder with properties retrieved from Viper.
func (b *Builder) InitFromViper(v *viper.Viper) *Builder {
for _, processor := range defaultProcessors {
prefix := fmt.Sprintf("processor.%s-%s.", processor.model, processor.protocol)
prefix := fmt.Sprintf(processorPrefixFmt, processor.model, processor.protocol)
p := &ProcessorConfiguration{Model: processor.model, Protocol: processor.protocol}
p.Workers = v.GetInt(prefix + suffixWorkers)
p.Server.QueueSize = v.GetInt(prefix + suffixServerQueueSize)
Expand Down
8 changes: 7 additions & 1 deletion cmd/agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"
"github.com/uber/jaeger-lib/metrics"
jexpvar "github.com/uber/jaeger-lib/metrics/expvar"
"github.com/uber/jaeger-lib/metrics/fork"
_ "go.uber.org/automaxprocs"
"go.uber.org/zap"

Expand Down Expand Up @@ -49,9 +51,12 @@ func main() {
return err
}
logger := svc.Logger // shortcut
mFactory := svc.MetricsFactory.
baseFactory := svc.MetricsFactory.
Namespace(metrics.NSOptions{Name: "jaeger"}).
Namespace(metrics.NSOptions{Name: "agent"})
mFactory := fork.New("internal",
jexpvar.NewFactory(10), // backend for internal opts
baseFactory)

rOpts := new(reporter.Options).InitFromViper(v, logger)
grpcBuilder := grpc.NewConnBuilder().InitFromViper(v)
Expand Down Expand Up @@ -79,6 +84,7 @@ func main() {
if err := agent.Run(); err != nil {
return fmt.Errorf("failed to run the agent: %w", err)
}

svc.RunAndThen(func() {
agent.Stop()
cp.Close()
Expand Down
7 changes: 6 additions & 1 deletion cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
jaegerClientConfig "github.com/uber/jaeger-client-go/config"
jaegerClientZapLog "github.com/uber/jaeger-client-go/log/zap"
"github.com/uber/jaeger-lib/metrics"
jexpvar "github.com/uber/jaeger-lib/metrics/expvar"
"github.com/uber/jaeger-lib/metrics/fork"
_ "go.uber.org/automaxprocs"
"go.uber.org/zap"

Expand Down Expand Up @@ -81,7 +83,10 @@ by default uses only in-memory database.`,
}
logger := svc.Logger // shortcut
rootMetricsFactory := svc.MetricsFactory // shortcut
metricsFactory := rootMetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
metricsFactory := fork.New("internal",
jexpvar.NewFactory(10), // backend for internal opts
rootMetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"}))

tracerCloser := initTracer(rootMetricsFactory, svc.Logger)

storageFactory.InitFromViper(v)
Expand Down
1 change: 1 addition & 0 deletions cmd/collector/app/builder_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,5 +116,6 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions {
cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins)
cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders)
cOpts.TLS = tlsFlagsConfig.InitFromViper(v)

return cOpts
}
7 changes: 7 additions & 0 deletions cmd/collector/app/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,17 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error {
} else {
c.zkServer = zkServer
}
c.publishOpts(builderOpts)

return nil
}

func (c *Collector) publishOpts(cOpts *CollectorOptions) {
internalFactory := c.metricsFactory.Namespace(metrics.NSOptions{Name: "internal"})
internalFactory.Gauge(metrics.Options{Name: collectorNumWorkers}).Update(int64(cOpts.NumWorkers))
internalFactory.Gauge(metrics.Options{Name: collectorQueueSize}).Update(int64(cOpts.QueueSize))
}

// Close the component and all its underlying dependencies
func (c *Collector) Close() error {
// gRPC server
Expand Down
37 changes: 37 additions & 0 deletions cmd/collector/app/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/uber/jaeger-lib/metrics/fork"
"github.com/uber/jaeger-lib/metrics/metricstest"
"go.uber.org/zap"

Expand Down Expand Up @@ -61,3 +62,39 @@ type mockStrategyStore struct {
func (m *mockStrategyStore) GetSamplingStrategy(_ context.Context, serviceName string) (*sampling.SamplingStrategyResponse, error) {
return &sampling.SamplingStrategyResponse{}, nil
}

func TestCollector_PublishOpts(t *testing.T) {
// prepare
hc := healthcheck.New()
logger := zap.NewNop()
baseMetrics := metricstest.NewFactory(time.Second)
forkFactory := metricstest.NewFactory(time.Second)
metricsFactory := fork.New("internal", forkFactory, baseMetrics)
spanWriter := &fakeSpanWriter{}
strategyStore := &mockStrategyStore{}

c := New(&CollectorParams{
ServiceName: "collector",
Logger: logger,
MetricsFactory: metricsFactory,
SpanWriter: spanWriter,
StrategyStore: strategyStore,
HealthCheck: hc,
})
collectorOpts := &CollectorOptions{
NumWorkers: 24,
QueueSize: 42,
}

c.Start(collectorOpts)
defer c.Close()

forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
Name: "internal.collector.num-workers",
Value: 24,
})
forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
Name: "internal.collector.queue-size",
Value: 42,
})
}
6 changes: 5 additions & 1 deletion cmd/collector/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"
"github.com/uber/jaeger-lib/metrics"
jexpvar "github.com/uber/jaeger-lib/metrics/expvar"
"github.com/uber/jaeger-lib/metrics/fork"
_ "go.uber.org/automaxprocs"
"go.uber.org/zap"

Expand Down Expand Up @@ -63,7 +65,9 @@ func main() {
}
logger := svc.Logger // shortcut
baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "collector"})
metricsFactory := fork.New("internal",
jexpvar.NewFactory(10), // backend for internal opts
baseFactory.Namespace(metrics.NSOptions{Name: "collector"}))

storageFactory.InitFromViper(v)
if err := storageFactory.Initialize(baseFactory, logger); err != nil {
Expand Down
8 changes: 8 additions & 0 deletions cmd/flags/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package flags

import (
"expvar"
"flag"
"fmt"
"os"
Expand Down Expand Up @@ -116,6 +117,13 @@ func (s *Service) Start(v *viper.Viper) error {
s.Logger.Info("Mounting metrics handler on admin server", zap.String("route", route))
s.Admin.Handle(route, h)
}

// Mount expvar routes on different backends
if metricsBuilder.Backend != "expvar" {
s.Logger.Info("Mounting expvar handler on admin server", zap.String("route", "/debug/vars"))
s.Admin.Handle("/debug/vars", expvar.Handler())
}

if err := s.Admin.Serve(); err != nil {
return fmt.Errorf("cannot start the admin server: %w", err)
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/opentelemetry/app/exporter/cassandraexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/exporter/exporterhelper"

collector_app "github.com/jaegertracing/jaeger/cmd/collector/app"
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
)

Expand Down Expand Up @@ -54,14 +55,18 @@ func (Factory) Type() configmodels.Type {
// This function implements OTEL component.ExporterFactoryBase interface.
func (f Factory) CreateDefaultConfig() configmodels.Exporter {
opts := f.OptionsFactory()
queueSettings := exporterhelper.CreateDefaultQueueSettings()
queueSettings.NumConsumers = collector_app.DefaultNumWorkers
queueSettings.QueueSize = collector_app.DefaultQueueSize

return &Config{
ExporterSettings: configmodels.ExporterSettings{
TypeVal: TypeStr,
NameVal: TypeStr,
},
TimeoutSettings: exporterhelper.CreateDefaultTimeoutSettings(),
RetrySettings: exporterhelper.CreateDefaultRetrySettings(),
QueueSettings: exporterhelper.CreateDefaultQueueSettings(),
QueueSettings: queueSettings,
Options: *opts,
}
}
Expand Down
Loading

0 comments on commit 91d0746

Please sign in to comment.