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

Update datadog tracer config proto and preserve remote_config default behavior for envoy v1.31 (#10145) #10191

Merged
merged 16 commits into from
Oct 11, 2024
Merged

Update datadog tracer config proto and preserve remote_config default behavior for envoy v1.31 (#10145) #10191

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
1df9537
Update datadog tracer config proto and preserve remote_config default…
andy-fong Oct 7, 2024
f5401e3
fixed typo in comment
andy-fong Oct 8, 2024
634758e
added changelog
andy-fong Oct 8, 2024
c061dba
revert formatting fix for generated code
andy-fong Oct 8, 2024
df8790d
used ptypes package instead
andy-fong Oct 8, 2024
963f2b7
Merge refs/heads/main into andyfong/datadog-proto-update
soloio-bulldozer[bot] Oct 9, 2024
f202bed
Merge refs/heads/main into andyfong/datadog-proto-update
soloio-bulldozer[bot] Oct 10, 2024
0680dac
Merge refs/heads/main into andyfong/datadog-proto-update
soloio-bulldozer[bot] Oct 10, 2024
b03b994
updated to use envoy-gloo 1.31.2-patch1
andy-fong Oct 10, 2024
ecd76ed
Adding changelog file to new location
Oct 10, 2024
afe8cb0
Deleting changelog file from old location
Oct 10, 2024
de22469
updated local_ratelimit_test for the removal of x-ratelimit-reset hea…
andy-fong Oct 10, 2024
85162b7
updated changelog
andy-fong Oct 10, 2024
6c1514c
Merge refs/heads/main into andyfong/datadog-proto-update
soloio-bulldozer[bot] Oct 11, 2024
d26879e
Merge refs/heads/main into andyfong/datadog-proto-update
soloio-bulldozer[bot] Oct 11, 2024
7a5e4e1
Merge refs/heads/main into andyfong/datadog-proto-update
soloio-bulldozer[bot] Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ SOURCES := $(shell find . -name "*.go" | grep -v test.go)
# for more information, see https://github.com/solo-io/gloo/pull/9633
# and
# https://soloio.slab.com/posts/extended-http-methods-design-doc-40j7pjeu
ENVOY_GLOO_IMAGE ?= quay.io/solo-io/envoy-gloo:1.30.4-patch2
ENVOY_GLOO_IMAGE ?= quay.io/solo-io/envoy-gloo:1.31.2-patch1
LDFLAGS := "-X github.com/solo-io/gloo/pkg/version.Version=$(VERSION)"
GCFLAGS ?=

Expand Down
14 changes: 14 additions & 0 deletions changelog/v1.18.0-beta27/update-datadog-tracer-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
changelog:
- type: NEW_FEATURE
issueLink: https://github.com/solo-io/gloo/issues/10145
resolvesIssue: false
description: >-
Updated the datadog tracer config proto to match envoy v1.31. New fields include
`collector_hostname` and `remote_config`. Remote config can now be configured or
disabled completely.
nfuden marked this conversation as resolved.
Show resolved Hide resolved
- type: DEPENDENCY_BUMP
dependencyOwner: solo-io
dependencyRepo: envoy-gloo
dependencyTag: 1.31.2-patch1
description: >-
Bump envoy-gloo to upgrade to envoy v1.31.2
25 changes: 25 additions & 0 deletions ...lo-io/gloo/projects/gloo/api/external/envoy/config/trace/v3/datadog.proto.sk.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions docs/data/ProtoMap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,9 @@ apis:
solo.io.envoy.config.trace.v3.DatadogConfig:
relativepath: reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/config/trace/v3/datadog.proto.sk/#DatadogConfig
package: solo.io.envoy.config.trace.v3
solo.io.envoy.config.trace.v3.DatadogRemoteConfig:
relativepath: reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/config/trace/v3/datadog.proto.sk/#DatadogRemoteConfig
package: solo.io.envoy.config.trace.v3
solo.io.envoy.config.trace.v3.OpenCensusConfig:
relativepath: reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/config/trace/v3/opencensus.proto.sk/#OpenCensusConfig
package: solo.io.envoy.config.trace.v3
Expand Down
30 changes: 30 additions & 0 deletions install/helm/gloo/crds/gateway.solo.io_v1_Gateway.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,13 +988,23 @@ spec:
properties:
clusterName:
type: string
collectorHostname:
type: string
collectorUpstreamRef:
properties:
name:
type: string
namespace:
type: string
type: object
remoteConfig:
properties:
disabled:
nullable: true
type: boolean
pollingInterval:
type: string
type: object
serviceName:
nullable: true
type: string
Expand Down Expand Up @@ -1627,13 +1637,23 @@ spec:
properties:
clusterName:
type: string
collectorHostname:
type: string
collectorUpstreamRef:
properties:
name:
type: string
namespace:
type: string
type: object
remoteConfig:
properties:
disabled:
nullable: true
type: boolean
pollingInterval:
type: string
type: object
serviceName:
nullable: true
type: string
Expand Down Expand Up @@ -2932,13 +2952,23 @@ spec:
properties:
clusterName:
type: string
collectorHostname:
type: string
collectorUpstreamRef:
properties:
name:
type: string
namespace:
type: string
type: object
remoteConfig:
properties:
disabled:
nullable: true
type: boolean
pollingInterval:
type: string
type: object
serviceName:
nullable: true
type: string
Expand Down
10 changes: 10 additions & 0 deletions install/helm/gloo/crds/gateway.solo.io_v1_HttpListenerOption.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,13 +979,23 @@ spec:
properties:
clusterName:
type: string
collectorHostname:
type: string
collectorUpstreamRef:
properties:
name:
type: string
namespace:
type: string
type: object
remoteConfig:
properties:
disabled:
nullable: true
type: boolean
pollingInterval:
type: string
type: object
serviceName:
nullable: true
type: string
Expand Down
10 changes: 10 additions & 0 deletions install/helm/gloo/crds/gateway.solo.io_v1_MatchableHttpGateway.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,13 +982,23 @@ spec:
properties:
clusterName:
type: string
collectorHostname:
type: string
collectorUpstreamRef:
properties:
name:
type: string
namespace:
type: string
type: object
remoteConfig:
properties:
disabled:
nullable: true
type: boolean
pollingInterval:
type: string
type: object
serviceName:
nullable: true
type: string
Expand Down
22 changes: 20 additions & 2 deletions pkg/utils/api_conversion/trace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,27 @@ var (

func ToEnvoyDatadogConfiguration(glooDatadogConfig *envoytracegloo.DatadogConfig, clusterName string) (*envoytrace.DatadogConfig, error) {
envoyDatadogConfig := &envoytrace.DatadogConfig{
CollectorCluster: clusterName,
ServiceName: glooDatadogConfig.GetServiceName().GetValue(),
CollectorCluster: clusterName,
ServiceName: glooDatadogConfig.GetServiceName().GetValue(),
CollectorHostname: glooDatadogConfig.GetCollectorHostname(),
}

remoteConfig := glooDatadogConfig.GetRemoteConfig()
if remoteConfig == nil {
// An empty RemoteConfig object on the envoy side means enabling RemoteConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a super helpful comment!

// RemoteConfig was enabled by default before envoy v1.31 and could not be turned off
// Here, we are trying to maintain this behavior for existing customer who might rely on this
// and not aware that they need to set this object
envoyDatadogConfig.RemoteConfig = &envoytrace.DatadogRemoteConfig{}
} else if !remoteConfig.GetDisabled().GetValue() {
// This Disabled field does not exist on the envoy side. We added it because our default
// is to enable when remoteConfig is not set to maintain backward compatibility. So, we need
// this field to disable if desire.
envoyDatadogConfig.RemoteConfig = &envoytrace.DatadogRemoteConfig{
PollingInterval: glooDatadogConfig.GetRemoteConfig().GetPollingInterval(),
}
} // Leaving RemoteConfig not set means it's disabled on the envoy side.
sam-heilbron marked this conversation as resolved.
Show resolved Hide resolved

return envoyDatadogConfig, nil
}

Expand Down
24 changes: 24 additions & 0 deletions projects/gloo/api/external/envoy/config/trace/v3/datadog.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ syntax = "proto3";
package solo.io.envoy.config.trace.v3;

import "google/protobuf/wrappers.proto";
import "google/protobuf/duration.proto";

import "udpa/annotations/migrate.proto";
import "udpa/annotations/status.proto";
Expand All @@ -19,6 +20,18 @@ option (solo.io.udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: Datadog tracer]

// Configuration for the Remote Configuration feature.
message DatadogRemoteConfig {
// Frequency at which new configuration updates are queried.
// If no value is provided, the default value is delegated to the Datadog tracing library.
google.protobuf.Duration polling_interval = 1;

// Disabled remote config.
// This field does not exist in envoy's config but allow us to preserve the default behavior
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Since this is our API, and we are exposing this specifically for backwards compatibility, should we clarify what the default behavior is?

Copy link
Contributor Author

@andy-fong andy-fong Oct 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If have other updates I need to push, I will clarify that. However, in general, because all the proto field default to false, 0 or "". The implied default behavior is disabled=false. That's the reason to pick disabled as the field instead of enabled (In general, I really don't like anything that's double negative to mean positive coming from the C++ world but I was taught that in go and protobuf, rely on the default value instead.)

// when upgrading to envoy v1.31
google.protobuf.BoolValue disabled = 2;
}

// Configuration for the Datadog tracer.
// [#extension: envoy.tracers.datadog]
message DatadogConfig {
Expand All @@ -38,6 +51,17 @@ message DatadogConfig {

// The name used for the service when traces are generated by envoy.
google.protobuf.StringValue service_name = 2 [(validate.rules).string = {min_len: 1}];

// Optional hostname to use when sending spans to the collector_cluster. Useful for collectors
// that require a specific hostname. Defaults to :ref:`collector_cluster <envoy_v3_api_field_config.trace.v3.DatadogConfig.collector_cluster>` above.
string collector_hostname = 4;

// Configures remote configuration.
// Remote Configuration allows to configure the tracer from Datadog's user interface.
// This feature can drastically increase the number of connections to the Datadog Agent.
// Each tracer regularly polls for configuration updates, and the number of tracers is the product
// of the number of listeners and worker threads.
DatadogRemoteConfig remote_config = 5;
}
option go_package = "github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/config/trace/v3";
import "extproto/ext.proto";
Expand Down
33 changes: 33 additions & 0 deletions projects/gloo/pkg/api/external/envoy/config/trace/v3/datadog.pb.clone.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading