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.

Sign up for GitHub

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

Jump to bottom

feat(otelcol/extension/jaeger_remote_sampling): resync defaults with upstreams #6763

Closed
wants to merge 4 commits into from
Closed

feat(otelcol/extension/jaeger_remote_sampling): resync defaults with upstreams #6763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2cb89c
feat(otelcol/extension/jaeger_remote_sampling): resync defaults with …
hainenber Mar 24, 2024
439aab9
Merge branch 'main' into resync-defaults-for-jaeger-remote-sampling
hainenber Mar 24, 2024
a5e3a36
chore(doc): update CHANGELOG entry
hainenber Mar 24, 2024
4fc0733
fix(otelcol/extension/jaeger_remote_sampling): resync unit tests
hainenber Mar 24, 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
8 changes: 5 additions & 3 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ Main (unreleased)

- Clustering for Grafana Agent in Flow mode has graduated from beta to stable.

- Resync defaults for `otelcol.processor.k8sattributes` with upstream. (@hainenber)

- Resync defaults for `otelcol.exporter.otlp` and `otelcol.exporter.otlphttp` with upstream. (@hainenber)
- Resync defaults to upstream for following OpenTelemetry-based components. (@hainenber)
- `otelcol.processor.k8sattributes`
- `otelcol.exporter.otlp`
- `otelcol.exporter.otlphttp`
- `otelcol.extension.jaeger_remote_sampling`

v0.40.3 (2024-03-14)
--------------------
Expand Down
8 changes: 4 additions & 4 deletions ...ent/otelcol/extension/jaeger_remote_sampling/internal/jaegerremotesampling/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func TestLoadConfig(t *testing.T) {
{
id: component.NewID(typeStr),
expected: &Config{
HTTPServerConfig: &confighttp.ServerConfig{Endpoint: ":5778"},
HTTPServerConfig: &confighttp.ServerConfig{Endpoint: "localhost:5778"},
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be surprising to users, as suddenly telemetry might be not be able to get pushed anymore.

Looking at the upstream docs I think the default is 0.0.0.0 for this reason. Can we align it with that value?

Once we start supporting OTel Collector feature gates (eg. such as component.UseLocalHostAsDefaultHost) then it might make sense to also support this.

Copy link
Member

Choose a reason for hiding this comment

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

@tpaschalis If you look at this discussion upstream, they're working towards changing the default to 127.0.0.1 in a future release. Currently if you enable a specific feature gate (UseLocalHostAsDefaultHost) then all the components default to 127.0.0.1 instead.

I think we need to figure out if we're going to do this before 1.0, since it would be a breaking change when it does get implememnted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is pretty breaking, maybe we can park this one and accept the deviance as part of grafana/agent features. Not sure how we can reconcile with upstream for this one.

GRPCServerConfig: &configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{
Endpoint: ":14250",
Endpoint: "localhost:14250",
Transport: "tcp",
}},
Source: Source{
Expand All @@ -42,9 +42,9 @@ func TestLoadConfig(t *testing.T) {
{
id: component.NewIDWithName(typeStr, "1"),
expected: &Config{
HTTPServerConfig: &confighttp.ServerConfig{Endpoint: ":5778"},
HTTPServerConfig: &confighttp.ServerConfig{Endpoint: "localhost:5778"},
GRPCServerConfig: &configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{
Endpoint: ":14250",
Endpoint: "localhost:14250",
Transport: "tcp",
}},
Source: Source{
Expand Down
9 changes: 4 additions & 5 deletions ...mponent/otelcol/extension/jaeger_remote_sampling/internal/jaegerremotesampling/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"context"
"sync"

"go.opentelemetry.io/collector/component"
otelcomponent "go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
Expand All @@ -31,14 +30,14 @@ func NewFactory() extension.Factory {
)
}

func createDefaultConfig() component.Config {
func createDefaultConfig() otelcomponent.Config {
return &Config{
HTTPServerConfig: &confighttp.ServerConfig{
Endpoint: ":5778",
Endpoint: "localhost:5778",
},
GRPCServerConfig: &configgrpc.ServerConfig{
NetAddr: confignet.AddrConfig{
Endpoint: ":14250",
Endpoint: "localhost:14250",
Transport: "tcp",
},
},
Expand All @@ -64,7 +63,7 @@ func logDeprecation(logger *zap.Logger) {
// featuregate.WithRegisterToVersion("0.92.0"),
// )

func createExtension(_ context.Context, set extension.CreateSettings, cfg component.Config) (extension.Extension, error) {
func createExtension(_ context.Context, set extension.CreateSettings, cfg otelcomponent.Config) (extension.Extension, error) {
logDeprecation(set.Logger)
return newExtension(cfg.(*Config), set.TelemetrySettings), nil
}
4 changes: 2 additions & 2 deletions ...nt/otelcol/extension/jaeger_remote_sampling/internal/jaegerremotesampling/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import (
func TestCreateDefaultConfig(t *testing.T) {
// prepare and test
expected := &Config{
HTTPServerConfig: &confighttp.ServerConfig{Endpoint: ":5778"},
HTTPServerConfig: &confighttp.ServerConfig{Endpoint: "localhost:5778"},
GRPCServerConfig: &configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{
Endpoint: ":14250",
Endpoint: "localhost:14250",
Transport: "tcp",
}},
}
Expand Down