Skip to content

Commit

Permalink
xds/internal/xdsclient: Emit unknown for CSM Labels if not present in…
Browse files Browse the repository at this point in the history
… CDS (grpc#7309)
  • Loading branch information
zasweq committed Jun 10, 2024
1 parent f8d04cc commit 8dee271
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 29 deletions.
21 changes: 21 additions & 0 deletions xds/internal/balancer/cdsbalancer/aggregate_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"google.golang.org/grpc/internal/testutils/xds/e2e"
"google.golang.org/grpc/serviceconfig"
"google.golang.org/grpc/status"
"google.golang.org/grpc/xds/internal"
"google.golang.org/grpc/xds/internal/balancer/clusterresolver"

v3clusterpb "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
Expand Down Expand Up @@ -85,6 +86,7 @@ func (s) TestAggregateClusterSuccess_LeafNode(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
},
Expand All @@ -94,6 +96,7 @@ func (s) TestAggregateClusterSuccess_LeafNode(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName + "-new",
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
},
Expand All @@ -108,6 +111,7 @@ func (s) TestAggregateClusterSuccess_LeafNode(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeLogicalDNS,
DNSHostname: "dns_host:8080",
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
},
Expand All @@ -117,6 +121,7 @@ func (s) TestAggregateClusterSuccess_LeafNode(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeLogicalDNS,
DNSHostname: "dns_host_new:8080",
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
},
Expand Down Expand Up @@ -211,12 +216,14 @@ func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
{
Cluster: dnsClusterName,
Type: clusterresolver.DiscoveryMechanismTypeLogicalDNS,
DNSHostname: fmt.Sprintf("%s:%d", dnsHostName, dnsPort),
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
Expand Down Expand Up @@ -247,12 +254,14 @@ func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
{
Cluster: dnsClusterNameNew,
Type: clusterresolver.DiscoveryMechanismTypeLogicalDNS,
DNSHostname: fmt.Sprintf("%s:%d", dnsHostNameNew, dnsPort),
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
Expand Down Expand Up @@ -298,12 +307,14 @@ func (s) TestAggregateClusterSuccess_ThenChangeRootToEDS(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
{
Cluster: dnsClusterName,
Type: clusterresolver.DiscoveryMechanismTypeLogicalDNS,
DNSHostname: fmt.Sprintf("%s:%d", dnsHostName, dnsPort),
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
Expand All @@ -329,6 +340,7 @@ func (s) TestAggregateClusterSuccess_ThenChangeRootToEDS(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down Expand Up @@ -363,6 +375,7 @@ func (s) TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate(t *testing.T
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down Expand Up @@ -391,12 +404,14 @@ func (s) TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate(t *testing.T
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
{
Cluster: dnsClusterName,
Type: clusterresolver.DiscoveryMechanismTypeLogicalDNS,
DNSHostname: fmt.Sprintf("%s:%d", dnsHostName, dnsPort),
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
Expand All @@ -420,6 +435,7 @@ func (s) TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate(t *testing.T
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down Expand Up @@ -572,6 +588,7 @@ func (s) TestAggregatedClusterSuccess_DiamondDependency(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down Expand Up @@ -639,12 +656,14 @@ func (s) TestAggregatedClusterSuccess_IgnoreDups(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
{
Cluster: clusterNameD,
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
},
},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
Expand Down Expand Up @@ -727,6 +746,7 @@ func (s) TestAggregatedCluster_NodeChildOfItself(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down Expand Up @@ -832,6 +852,7 @@ func (s) TestAggregatedCluster_CycleWithLeafNode(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: internal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down
6 changes: 6 additions & 0 deletions xds/internal/balancer/cdsbalancer/cdsbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"google.golang.org/grpc/resolver/manual"
"google.golang.org/grpc/serviceconfig"
"google.golang.org/grpc/status"
xdsinternal "google.golang.org/grpc/xds/internal"
"google.golang.org/grpc/xds/internal/balancer/clusterresolver"
"google.golang.org/grpc/xds/internal/xdsclient"
"google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
Expand Down Expand Up @@ -456,6 +457,7 @@ func (s) TestClusterUpdate_Success(t *testing.T) {
EDSServiceName: serviceName,
MaxConcurrentRequests: newUint32(512),
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: xdsinternal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
},
Expand Down Expand Up @@ -483,6 +485,7 @@ func (s) TestClusterUpdate_Success(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: xdsinternal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"ring_hash_experimental": {"minRingSize":100, "maxRingSize":1000}}]`),
},
Expand All @@ -505,6 +508,7 @@ func (s) TestClusterUpdate_Success(t *testing.T) {
Type: clusterresolver.DiscoveryMechanismTypeEDS,
EDSServiceName: serviceName,
OutlierDetection: json.RawMessage(`{"successRateEjection":{}}`),
TelemetryLabels: xdsinternal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"ring_hash_experimental": {"minRingSize":1024, "maxRingSize":8388608}}]`),
},
Expand Down Expand Up @@ -557,6 +561,7 @@ func (s) TestClusterUpdate_Success(t *testing.T) {
"requestVolume": 50
}
}`),
TelemetryLabels: xdsinternal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"ring_hash_experimental": {"minRingSize":1024, "maxRingSize":8388608}}]`),
},
Expand Down Expand Up @@ -607,6 +612,7 @@ func (s) TestClusterUpdate_SuccessWithLRS(t *testing.T) {
Creds: bootstrap.ChannelCreds{Type: "insecure"},
},
OutlierDetection: json.RawMessage(`{}`),
TelemetryLabels: xdsinternal.UnknownCSMLabels,
}},
XDSLBPolicy: json.RawMessage(`[{"xds_wrr_locality_experimental": {"childPolicy": [{"round_robin": {}}]}}]`),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"google.golang.org/grpc/resolver/manual"
"google.golang.org/grpc/serviceconfig"
"google.golang.org/grpc/status"
xdsinternal "google.golang.org/grpc/xds/internal"
"google.golang.org/grpc/xds/internal/balancer/clusterimpl"
"google.golang.org/grpc/xds/internal/balancer/outlierdetection"
"google.golang.org/grpc/xds/internal/balancer/priority"
Expand Down Expand Up @@ -400,8 +401,9 @@ func (s) TestOutlierDetectionConfigPropagationToChildPolicy(t *testing.T) {
ChildPolicy: &iserviceconfig.BalancerConfig{
Name: clusterimpl.Name,
Config: &clusterimpl.LBConfig{
Cluster: clusterName,
EDSServiceName: edsServiceName,
Cluster: clusterName,
EDSServiceName: edsServiceName,
TelemetryLabels: xdsinternal.UnknownCSMLabels,
ChildPolicy: &iserviceconfig.BalancerConfig{
Name: wrrlocality.Name,
Config: &wrrlocality.LBConfig{
Expand Down
7 changes: 7 additions & 0 deletions xds/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,10 @@ func SetLocalityID(addr resolver.Address, l LocalityID) resolver.Address {

// ResourceTypeMapForTesting maps TypeUrl to corresponding ResourceType.
var ResourceTypeMapForTesting map[string]any

// UnknownCSMLabels are TelemetryLabels emitted from CDS if CSM Telemetry Label
// data is not present in the CDS Resource.
var UnknownCSMLabels = map[string]string{
"csm.service_name": "unknown",
"csm.service_namespace_name": "unknown",
}
2 changes: 1 addition & 1 deletion xds/internal/xdsclient/tests/cds_watchers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func verifyClusterUpdate(ctx context.Context, updateCh *testutils.Channel, wantU
return fmt.Errorf("received update with error type %v, want %v", gotType, wantType)
}
}
cmpOpts := []cmp.Option{cmpopts.EquateEmpty(), cmpopts.IgnoreFields(xdsresource.ClusterUpdate{}, "Raw", "LBPolicy")}
cmpOpts := []cmp.Option{cmpopts.EquateEmpty(), cmpopts.IgnoreFields(xdsresource.ClusterUpdate{}, "Raw", "LBPolicy", "TelemetryLabels")}
if diff := cmp.Diff(wantUpdate.update, got.update, cmpOpts...); diff != "" {
return fmt.Errorf("received unexpected diff in the cluster resource update: (-want, got):\n%s", diff)
}
Expand Down
2 changes: 1 addition & 1 deletion xds/internal/xdsclient/tests/resource_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ func (s) TestHandleClusterResponseFromManagementServer(t *testing.T) {
}
cmpOpts := []cmp.Option{
cmpopts.EquateEmpty(),
cmpopts.IgnoreFields(xdsresource.ClusterUpdate{}, "Raw", "LBPolicy"),
cmpopts.IgnoreFields(xdsresource.ClusterUpdate{}, "Raw", "LBPolicy", "TelemetryLabels"),
}
if diff := cmp.Diff(test.wantUpdate, gotUpdate, cmpOpts...); diff != "" {
t.Fatalf("Unexpected diff in metadata, diff (-want +got):\n%s", diff)
Expand Down
Loading

0 comments on commit 8dee271

Please sign in to comment.