Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jaeger-v2] Add remotesampling extension #5389

Draft
wants to merge 39 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
1764c4c
init
Pushkarm029 Apr 25, 2024
ac859c7
fix
Pushkarm029 Apr 25, 2024
0b53417
added README.md
Pushkarm029 Apr 25, 2024
a7ba921
Merge branch 'main' into sampling_extension
Pushkarm029 Apr 25, 2024
5f267d0
updated README.md
Pushkarm029 Apr 26, 2024
901f76a
fix
Pushkarm029 Apr 26, 2024
7a03553
fix
Pushkarm029 Apr 26, 2024
36f38d2
Merge branch 'main' into sampling_extension
Pushkarm029 Apr 26, 2024
cb0d857
[e2e tests] Ping v2 binary to be ready before running tests (#5382)
dependabot[bot] Apr 26, 2024
67f2203
[agent] Use grpc.NewClient (#5392)
yurishkuro Apr 27, 2024
9fcf052
Bump anchore/sbom-action from 0.15.10 to 0.15.11 (#5395)
dependabot[bot] Apr 29, 2024
976dfb8
[jaeger-v2] Normalize config names (#5400)
yurishkuro Apr 30, 2024
cf8fe8d
Combine jaeger UI release notes with jaeger backend (#5405)
albertteoh May 1, 2024
a78782e
[jaeger-v2] Define an internal interface of storage v2 spanstore (#5399)
james-ryans May 1, 2024
6ba2c8a
Upgrade mockery from v2.14.0 to v2.42.3 (#5404)
Pushkarm029 May 1, 2024
487013e
Bump google.golang.org/protobuf from 1.33.0 to 1.34.0 (#5401)
dependabot[bot] May 1, 2024
98b5f99
Prepare releaes 1.57.0 (#5406)
albertteoh May 1, 2024
7c7aa94
Add `Purge` method for ES/OS (#5407)
Pushkarm029 May 2, 2024
d631ae9
[es] Remove unused indexCache (#5408)
yurishkuro May 2, 2024
96c4e33
[tests] Simplify integration tests (#5409)
yurishkuro May 3, 2024
a3a3f20
Only build Docker images for Crossdock tests for linux/amd64 (#5410)
varshith257 May 3, 2024
a15962b
Use helper action to retry codecov uploads (#5411)
yurishkuro May 3, 2024
fab02d8
[jaeger-v2] add elasticsearch & opensearch e2e integration test (#5345)
Pushkarm029 May 3, 2024
f1c253a
[v2] Add diagrams to the docs (#5412)
yurishkuro May 3, 2024
3579fe3
Add Purge method for cassandra (#5414)
akagami-harsh May 4, 2024
b4ab638
Add missing mermaid markup (#5413)
yurishkuro May 4, 2024
74b07a0
Merge branch 'main' into sampling_extension
Pushkarm029 May 4, 2024
fcc5547
major changes
Pushkarm029 May 6, 2024
8d4c4b1
some fixes
Pushkarm029 May 6, 2024
076eb58
minor fix
Pushkarm029 May 7, 2024
2912e9a
some progress
Pushkarm029 May 8, 2024
8fd879d
Merge branch 'main' into sampling_extension
Pushkarm029 May 10, 2024
bcd90b7
Merge branch 'main' into sampling_extension
Pushkarm029 May 22, 2024
eb029f7
some changes
Pushkarm029 May 22, 2024
1631830
fix
Pushkarm029 May 22, 2024
28f0dc9
experimental config sharing
Pushkarm029 May 22, 2024
f39093b
fix
Pushkarm029 May 24, 2024
54a20fa
minor fix
Pushkarm029 May 25, 2024
9fb0797
Merge branch 'main' into sampling_extension
Pushkarm029 May 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 15 additions & 2 deletions cmd/jaeger/config-badger.yaml
Pushkarm029 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
service:
extensions: [jaeger_storage, jaeger_query]
extensions: [jaeger_storage, jaeger_query, remote_sampling]
pipelines:
traces:
receivers: [otlp]
processors: [batch]
processors: [batch, adaptive_sampling]
exporters: [jaeger_storage_exporter]

extensions:
remote_sampling:
# You can either use file or adaptive sampling strategy in remote_sampling
# file:
# path: ./cmd/jaeger/sampling-strategies.json
Comment on lines +12 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should default to file, not to more complex adaptive?

adaptive:
Pushkarm029 marked this conversation as resolved.
Show resolved Hide resolved
initial_sampling_probability: 0.1
strategy_store: badger_main
http:
grpc:


jaeger_query:
trace_storage: badger_main
trace_storage_archive: badger_archive
Expand Down Expand Up @@ -37,6 +48,8 @@ receivers:

processors:
batch:
adaptive_sampling:
strategy_store: badger_main

exporters:
jaeger_storage_exporter:
Expand Down
17 changes: 15 additions & 2 deletions cmd/jaeger/config-cassandra.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
service:
extensions: [jaeger_storage, jaeger_query]
extensions: [jaeger_storage, jaeger_query, remote_sampling]
pipelines:
traces:
receivers: [otlp]
processors: [batch]
processors: [batch, adaptive_sampling]
exporters: [jaeger_storage_exporter]

extensions:
remote_sampling:
# You can either use file or adaptive sampling strategy in remote_sampling
# file:
# path: ./cmd/jaeger/sampling-strategies.json
adaptive:
initial_sampling_probability: 0.1
strategy_store: cassandra_main
http:
grpc:

jaeger_query:
trace_storage: cassandra_main
trace_storage_archive: cassandra_archive
Expand Down Expand Up @@ -47,6 +57,9 @@ receivers:

processors:
batch:
adaptive_sampling:
strategy_store: cassandra_main


exporters:
jaeger_storage_exporter:
Expand Down
16 changes: 14 additions & 2 deletions cmd/jaeger/config-elasticsearch.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
service:
extensions: [jaeger_storage, jaeger_query]
extensions: [jaeger_storage, jaeger_query, remote_sampling]
pipelines:
traces:
receivers: [otlp]
processors: [batch]
processors: [batch, adaptive_sampling]
exporters: [jaeger_storage_exporter]

extensions:
remote_sampling:
# You can either use file or adaptive sampling strategy in remote_sampling
# file:
# path: ./cmd/jaeger/sampling-strategies.json
adaptive:
initial_sampling_probability: 0.1
strategy_store: es_main
http:
grpc:

jaeger_query:
trace_storage: es_main
trace_storage_archive: es_archive
Expand Down Expand Up @@ -35,6 +45,8 @@ receivers:

processors:
batch:
adaptive_sampling:
strategy_store: es_main

exporters:
jaeger_storage_exporter:
Expand Down
16 changes: 14 additions & 2 deletions cmd/jaeger/config-opensearch.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
service:
extensions: [jaeger_storage, jaeger_query]
extensions: [jaeger_storage, jaeger_query, remote_sampling]
pipelines:
traces:
receivers: [otlp]
processors: [batch]
processors: [batch, adaptive_sampling]
exporters: [jaeger_storage_exporter]

extensions:
remote_sampling:
# You can either use file or adaptive sampling strategy in remote_sampling
# file:
# path: ./cmd/jaeger/sampling-strategies.json
adaptive:
initial_sampling_probability: 0.1
strategy_store: os_main
http:
grpc:

jaeger_query:
trace_storage: os_main
trace_storage_archive: os_archive
Expand Down Expand Up @@ -36,6 +46,8 @@ receivers:

processors:
batch:
adaptive_sampling:
strategy_store: os_main

exporters:
jaeger_storage_exporter:
Expand Down
16 changes: 14 additions & 2 deletions cmd/jaeger/config.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
service:
extensions: [jaeger_storage, jaeger_query]
extensions: [jaeger_storage, jaeger_query, remote_sampling]
pipelines:
traces:
receivers: [otlp, jaeger, zipkin]
processors: [batch]
processors: [batch, adaptive_sampling]
exporters: [jaeger_storage_exporter]

extensions:
Expand All @@ -13,6 +13,16 @@ extensions:
# zpages:
# endpoint: 0.0.0.0:55679

remote_sampling:
# You can either use file or adaptive sampling strategy in remote_sampling
# file:
# path: ./cmd/jaeger/sampling-strategies.json
adaptive:
initial_sampling_probability: 0.1
strategy_store: memstore
http:
grpc:

jaeger_query:
trace_storage: memstore
trace_storage_archive: memstore_archive
Expand Down Expand Up @@ -42,6 +52,8 @@ receivers:

processors:
batch:
adaptive_sampling:
strategy_store: memstore

exporters:
jaeger_storage_exporter:
Expand Down
6 changes: 4 additions & 2 deletions cmd/jaeger/internal/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import (
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/exporters/storageexporter"
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerquery"
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/remotesampling"
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/integration/storagecleaner"
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/processors/adaptivesampling"
)

type builders struct {
Expand Down Expand Up @@ -61,8 +63,8 @@ func (b builders) build() (otelcol.Factories, error) {
// add-ons
jaegerquery.NewFactory(),
jaegerstorage.NewFactory(),
remotesampling.NewFactory(),
storagecleaner.NewFactory(),
// TODO add adaptive sampling
)
if err != nil {
return otelcol.Factories{}, err
Expand Down Expand Up @@ -99,7 +101,7 @@ func (b builders) build() (otelcol.Factories, error) {
batchprocessor.NewFactory(),
memorylimiterprocessor.NewFactory(),
// add-ons
// TODO add adaptive sampling
adaptivesampling.NewFactory(),
)
if err != nil {
return otelcol.Factories{}, err
Expand Down
80 changes: 80 additions & 0 deletions cmd/jaeger/internal/extension/remotesampling/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package remotesampling

import (
"errors"
"reflect"
"time"

"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
)

var (
errNoSource = errors.New("no sampling strategy specified, has to be either 'adaptive' or 'file'")
errMultipleSource = errors.New("only one sampling strategy can be specified, has to be either 'adaptive' or 'file'")
)

type FileConfig struct {
// File specifies a local file as the strategies source
Path string `mapstructure:"path"`
}

type AdaptiveConfig struct {
Pushkarm029 marked this conversation as resolved.
Show resolved Hide resolved
// name of the strategy storage defined in the jaegerstorage extension
StrategyStore string `mapstructure:"strategy_store"`

// InitialSamplingProbability is the initial sampling probability for all new operations.
InitialSamplingProbability float64 `mapstructure:"initial_sampling_probability"`

// AggregationBuckets is the total number of aggregated throughput buckets kept in memory, ie. if
// the CalculationInterval is 1 minute (each bucket contains 1 minute of thoughput data) and the
// AggregationBuckets is 3, the adaptive sampling processor will keep at most 3 buckets in memory for
// all operations.
// TODO(wjang): Expand on why this is needed when BucketsForCalculation seems to suffice.
AggregationBuckets int `mapstructure:"aggregation_buckets"`

// MinSamplesPerSecond determines the min number of traces that are sampled per second.
// For example, if the value is 0.01666666666 (one every minute), then the sampling processor will do
// its best to sample at least one trace a minute for an operation. This is useful for low QPS operations
// that may never be sampled by the probabilistic sampler.
MinSamplesPerSecond float64 `mapstructure:"min_samples_per_second"`

// LeaderLeaseRefreshInterval is the duration to sleep if this processor is elected leader before
// attempting to renew the lease on the leader lock. NB. This should be less than FollowerLeaseRefreshInterval
// to reduce lock thrashing.
LeaderLeaseRefreshInterval time.Duration `mapstructure:"leader_lease_refresh_interval"`

// FollowerLeaseRefreshInterval is the duration to sleep if this processor is a follower
// (ie. failed to gain the leader lock).
FollowerLeaseRefreshInterval time.Duration `mapstructure:"follower_lease_refresh_interval"`
}

type Config struct {
File FileConfig `mapstructure:"file"`
Adaptive AdaptiveConfig `mapstructure:"adaptive"`
HTTP HTTPConfig `mapstructure:"http"`
GRPC GRPCConfig `mapstructure:"grpc"`
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we follow the same?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's do the same. We should define all 4 fields in the config as pointers, so that both File/Adaptive can be optional, and HTTP/GRPC can also be disabled independently.

We need to have a unit test for this behavior in the config, i.e. feed it different YAML strings and check that the corresponding sections are there or not there. I think we also have other configs that could use this pattern, I will take a look.

Lastly, the Validate function below should change to reflect this pointer change, the logic there can be different, e.g. instead of if cfg.File.Path != "" && cfg.Adaptive.StrategyStore != "" we should check if the whole struct is defined or not.

I also think the sub-structs should have their own Validate method that can be called from the main Validate if the struct is not nil. For example, if someone defines file: without any arguments we should raise an error that the config file name is missing (I think we can use struct tags to add these requirements).


type HTTPConfig struct {
confighttp.ServerConfig `mapstructure:",squash"`
}

type GRPCConfig struct {
configgrpc.ServerConfig `mapstructure:",squash"`
}

func (cfg *Config) Validate() error {
emptyCfg := createDefaultConfig().(*Config)
if reflect.DeepEqual(*cfg, *emptyCfg) {
return errNoSource
}

if cfg.File.Path != "" && cfg.Adaptive.StrategyStore != "" {
return errMultipleSource
}
return nil
}