Question about destination_service and destination_service_name metric labels

[delavet]

Please provide an in-depth description of the question you have:
I have found out a strange behavior about the metric labels exported by Kmesh. I'm testing in dual-engine mode. If you explore the tcp traffics and compare the destination_service and destination_service_name labels, you'll see that these two labels are exactly the same: they are Service FQDN.

What do you think about this question?:
Istio has explained these two metrics in the documentation about Istio Standard Metrics(https://istio.io/latest/docs/reference/config/metrics/).

Destination Service: This identifies destination service host responsible for an incoming request. Ex: details.default.svc.cluster.local.

Destination Service Name: This identifies the destination service name. Ex: details.

As for Kmesh, I'm not sure if this is a bug, or this is designed by purpose. Therefore I just submit a question here.

A known effection of this issue is that Kiali may draw Service nodes on its traffic graph depends on the destination_service_name label.

Environment:

• Kmesh version: latest
• Others:

[Kuromesi]

/assign

[LiZhenCheng9527]

kmesh/pkg/controller/telemetry/metric.go

Lines 424 to 449 in 1fca907

	if dstWorkload != nil {
	namespacedhost := ""
	for k, portList := range dstWorkload.Services {
	for _, port := range portList.Ports {
	if port.TargetPort == uint32(dstPort) {
	namespacedhost = k
	break
	}
	}
	if namespacedhost != "" {
	break
	}
	}
	svcHost := ""
	svcNamespace := ""
	if len(strings.Split(namespacedhost, "/")) == 2 {
	svcNamespace = strings.Split(namespacedhost, "/")[0]
	svcHost = strings.Split(namespacedhost, "/")[1]
	}
	trafficLabels.destinationService = svcHost
	trafficLabels.destinationServiceNamespace = svcNamespace
	trafficLabels.destinationServiceName = svcHost

Changing the code on this side should do the trick.

[Kuromesi]

kmesh/pkg/controller/telemetry/metric.go

Lines 424 to 449 in 1fca907

	if dstWorkload != nil {
	namespacedhost := ""
	for k, portList := range dstWorkload.Services {
	for _, port := range portList.Ports {
	if port.TargetPort == uint32(dstPort) {
	namespacedhost = k
	break
	}
	}
	if namespacedhost != "" {
	break
	}
	}
	svcHost := ""
	svcNamespace := ""
	if len(strings.Split(namespacedhost, "/")) == 2 {
	svcNamespace = strings.Split(namespacedhost, "/")[0]
	svcHost = strings.Split(namespacedhost, "/")[1]
	}
	trafficLabels.destinationService = svcHost
	trafficLabels.destinationServiceNamespace = svcNamespace
	trafficLabels.destinationServiceName = svcHost

Changing the code on this side should do the trick.

I'm wondering if we should also store serviceCache in the MetricController, so that we can index services from it?

[LiZhenCheng9527]

kmesh/pkg/controller/workload/cache/service_cache.go

Lines 34 to 40 in 1fca907

	type serviceCache struct {
	mutex sync.RWMutex
	// keyed by namespace/hostname->service
	servicesByResourceName map[string]*workloadapi.Service
	}

In controller.processer there is serviceCacha. Not sure if this will meet for your requests.
Or you want get service by workload?

[Kuromesi]

I'm trying to fix the issue by adding a serviceCache in MetricController and index services when buildServiceMetric, I'm not sure is it more reasonable?

type MetricController struct {
	EnableAccesslog     atomic.Bool
	EnableMonitoring    atomic.Bool
	workloadCache       cache.WorkloadCache
        // add service cache
	serviceCache       cache.ServiceCache
	workloadMetricCache map[workloadMetricLabels]*workloadMetricInfo
	serviceMetricCache  map[serviceMetricLabels]*serviceMetricInfo
	mutex               sync.RWMutex
}

// buildServiceMetric
namespacedhost := ""
for k, portList := range dstWorkload.Services {
	for _, port := range portList.Ports {
		if port.TargetPort == uint32(dstPort) {
			namespacedhost = k
			break
		}
	}
	if namespacedhost != "" {
		break
	}
}

svcHost := ""
svcNamespace := ""
if len(strings.Split(namespacedhost, "/")) == 2 {
	svcNamespace = strings.Split(namespacedhost, "/")[0]
	svcHost = strings.Split(namespacedhost, "/")[1]
}

// index service
svc := m.serviceCache.GetService(namespacedhost)
trafficLabels.destinationService = svc.HostName
trafficLabels.destinationServiceNamespace = svcNamespace
trafficLabels.destinationServiceName = svc.Name

[LiZhenCheng9527]

Yes, that does make more sense.