Skip to content

Commit

Permalink
Merge pull request #104 from xmidt-org/improveConfigs
Browse files Browse the repository at this point in the history
Improve Tracing Configurations
  • Loading branch information
renaz6 committed Mar 1, 2023
2 parents 1bd941a + 149d1fd commit d3d54e4
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Split up the tracing configuration options so user can decide exactly what gets traced and when [#104](https://github.com/xmidt-org/candlelight/pull/104)

## [v0.0.14]
- Updated traceProvider to only sample traces with remote parents (configurable) [#100](https://github.com/xmidt-org/candlelight/pull/100)
Expand Down
14 changes: 10 additions & 4 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,16 @@ type Config struct {
// TracerProvider.
Providers map[string]ProviderConstructor `json:"-"`

// SampleLocalTraces defines whether local traces will be created and exported.
// SampleLocalTraces = true if you want to create and export local traces. Otherwise,
// only remote traces/spans will be sampled.
SampleLocalTraces bool `json:"sampleLocalTraces"`
// ParentBased and NoParent dictate if and when new spans should be created.
// ParentBased = "ignore" (default), tracing is effectively turned off and the "NoParent" value is ignored
// ParentBased = "honor", the sampling decision is made by the parent of the span
ParentBased string `json:"parentBased"`

// NoParent decides if a root span should be initiated in the case where there is no existing parent
// This value is ignored if ParentBased = "ignore"
// NoParent = "never" (default), root spans are not initiated
// NoParent = "always", roots spans are initiated
NoParent string `json:"noParent"`
}

// TraceConfig will be used in TraceMiddleware to use config and TraceProvider
Expand Down
36 changes: 30 additions & 6 deletions tracerProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import (
var (
ErrTracerProviderNotFound = errors.New("TracerProvider builder could not be found")
ErrTracerProviderBuildFailed = errors.New("Failed building TracerProvider")
ErrInvalidParentBasedValue = errors.New("Invalid ParentBased value provided in configuration")
ErrInvalidNoParentValue = errors.New("Invalid No Parent value provided in configuration")
)

// DefaultTracerProvider is used when no provider is given.
Expand All @@ -57,20 +59,42 @@ func ConfigureTracerProvider(config Config) (trace.TracerProvider, error) {
// Handling camelcase of provider.
config.Provider = strings.ToLower(config.Provider)
providerConfig := config.Providers[config.Provider]
sampleLocalTraces := config.SampleLocalTraces
parentBasedTracing := config.ParentBased
noParentTracing := config.NoParent
if providerConfig == nil {
providerConfig = providersConfig[config.Provider]
}
if providerConfig == nil {
return nil, fmt.Errorf("%w for provider %s", ErrTracerProviderNotFound, config.Provider)
}

// Setting up trace sampler based on SampleLocalTraces value in config file
// Default behavior: never sample local traces
sampler := sdktrace.ParentBased(sdktrace.NeverSample())
// If parentBased value is empty, use default value
if parentBasedTracing == "" {
parentBasedTracing = "ignore"
}

// If noParent value is empty, use default value
if noParentTracing == "" {
noParentTracing = "never"
}

// Setting up trace sampler based on ParentBased and NoParent values in the config
var sampler sdktrace.Sampler

if sampleLocalTraces {
sampler = sdktrace.ParentBased(sdktrace.AlwaysSample())
if parentBasedTracing == "ignore" {
sampler = sdktrace.NeverSample()
} else if parentBasedTracing == "honor" {
if noParentTracing == "never" {
sampler = sdktrace.ParentBased(sdktrace.NeverSample())

} else if noParentTracing == "always" {
sampler = sdktrace.ParentBased(sdktrace.AlwaysSample())

} else {
return nil, ErrInvalidNoParentValue
}
} else {
return nil, ErrInvalidParentBasedValue
}

provider, err := providerConfig(config, sampler)
Expand Down
71 changes: 68 additions & 3 deletions tracerProvider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,82 @@ func TestConfigureTracerProvider(t *testing.T) {
{
Description: "Otlp/gRPC: Valid",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "ignore",
NoParent: "never",
},
},
{
Description: "Otlp/gRPC: Valid",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "honor",
NoParent: "never",
},
},
{
Description: "Otlp/gRPC: Valid",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "honor",
NoParent: "always",
},
},
{
Description: "Otlp/gRPC: Valid",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "ignore",
NoParent: "always",
},
},
{
Description: "Otlp/gRPC: Missing Endpoint",
Config: Config{
Provider: "otlp/grpc",
Provider: "otlp/grpc",
ParentBased: "ignore",
NoParent: "never",
},
Err: ErrTracerProviderBuildFailed,
},
{
Description: "Valid Missing ParentBased Value",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
},
},
{
Description: "Valid Missing NoParent Value",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "honor",
},
},
{
Description: "Invalid ParentBased Value",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "dishonor",
},
Err: ErrInvalidParentBasedValue,
},
{
Description: "Invalid No Parent Value",
Config: Config{
Provider: "otlp/grpc",
Endpoint: "http://localhost",
ParentBased: "honor",
NoParent: "sometimes",
},
Err: ErrInvalidNoParentValue,
},
{
Description: "Otlp/HTTP: Valid",
Config: Config{
Expand Down

0 comments on commit d3d54e4

Please sign in to comment.