From 396aa34012332c10d133a8d2c35e18b39affe21c Mon Sep 17 00:00:00 2001 From: Alon Braymok <138359965+alonkeyval@users.noreply.github.com> Date: Wed, 11 Dec 2024 18:49:14 +0200 Subject: [PATCH] Add healthy condition to source (#1976) This pull request includes several changes to the `frontend` package, focusing on refactoring functions, improving code readability, and enhancing the condition handling for instrumented applications. ### Refactoring and Code Simplification: * Removed the mapping of instrumentation options in the `instrumentedApplicationToActualSource` function in `frontend/graph/conversions.go` to simplify the code. * Changed the `AutoInstrumentedDecision` field to be an empty string in the `instrumentedApplicationToActualSource` function in `frontend/graph/conversions.go`. ### Condition Handling Enhancements: * Added a call to `AddHealthyInstrumentationInstancesCondition` in the `K8sActualSources` resolver in `frontend/graph/schema.resolvers.go` to ensure healthy instrumentation instances are tracked. * Modified the `GetActualSource` function in `frontend/services/sources.go` to use `AddHealthyInstrumentationInstancesCondition` with a `nil` source parameter. * Updated the `addHealthyInstrumentationInstancesCondition` function (renamed to `AddHealthyInstrumentationInstancesCondition`) in `frontend/services/sources.go` to use `model.ConditionStatusTrue` and `model.ConditionStatusFalse` instead of `metav1.ConditionTrue` and `metav1.ConditionFalse`, respectively. Also, added formatted message and transition time handling. [[1]](diffhunk://#diff-d8f62b6675961fc4e307e4fd622a59132ddc730b6e701ae1bc43dd1695f969a7L126-R126) [[2]](diffhunk://#diff-d8f62b6675961fc4e307e4fd622a59132ddc730b6e701ae1bc43dd1695f969a7L153-R164) ### Utility Functions: * Added a new utility function `Metav1TimeToString` in `frontend/services/utils.go` to convert `metav1.Time` to a string formatted in RFC3339. ### Minor Code Improvements: * Removed unnecessary comments in the `Actions` resolver in `frontend/graph/schema.resolvers.go` to improve code readability. * Added the `time` package import in `frontend/services/utils.go` for date and time formatting. --------- Co-authored-by: alonkeyval Co-authored-by: Ben Elferink --- frontend/graph/conversions.go | 83 +++--------------------------- frontend/graph/schema.resolvers.go | 17 ++---- frontend/services/sources.go | 77 +++------------------------ frontend/services/utils.go | 8 +++ 4 files changed, 26 insertions(+), 159 deletions(-) diff --git a/frontend/graph/conversions.go b/frontend/graph/conversions.go index a4cb29a9c..5667f3f30 100644 --- a/frontend/graph/conversions.go +++ b/frontend/graph/conversions.go @@ -5,7 +5,6 @@ import ( "github.com/odigos-io/odigos/api/odigos/v1alpha1" gqlmodel "github.com/odigos-io/odigos/frontend/graph/model" - "github.com/odigos-io/odigos/frontend/services" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -42,56 +41,6 @@ func k8sLastTransitionTimeToGql(t v1.Time) *string { return &str } -func k8sThinSourceToGql(k8sSource *services.ThinSource) *gqlmodel.K8sActualSource { - - hasInstrumentedApplication := k8sSource.IaDetails != nil - - var gqlIaDetails *gqlmodel.InstrumentedApplicationDetails - if hasInstrumentedApplication { - gqlIaDetails = &gqlmodel.InstrumentedApplicationDetails{ - Containers: make([]*gqlmodel.SourceContainerRuntimeDetails, len(k8sSource.IaDetails.Languages)), - Conditions: make([]*gqlmodel.Condition, len(k8sSource.IaDetails.Conditions)), - } - - for i, lang := range k8sSource.IaDetails.Languages { - gqlIaDetails.Containers[i] = &gqlmodel.SourceContainerRuntimeDetails{ - ContainerName: lang.ContainerName, - Language: lang.Language, - } - } - - for i, cond := range k8sSource.IaDetails.Conditions { - gqlIaDetails.Conditions[i] = &gqlmodel.Condition{ - Type: cond.Type, - Status: k8sConditionStatusToGql(cond.Status), - Reason: &cond.Reason, - LastTransitionTime: k8sLastTransitionTimeToGql(cond.LastTransitionTime), - Message: &cond.Message, - } - } - } - - return &gqlmodel.K8sActualSource{ - Namespace: k8sSource.Namespace, - Kind: k8sKindToGql(k8sSource.Kind), - Name: k8sSource.Name, - NumberOfInstances: &k8sSource.NumberOfRunningInstances, - InstrumentedApplicationDetails: gqlIaDetails, - } -} - -func k8sSourceToGql(k8sSource *services.Source) *gqlmodel.K8sActualSource { - baseSource := k8sThinSourceToGql(&k8sSource.ThinSource) - return &gqlmodel.K8sActualSource{ - Namespace: baseSource.Namespace, - Kind: baseSource.Kind, - Name: baseSource.Name, - NumberOfInstances: baseSource.NumberOfInstances, - InstrumentedApplicationDetails: baseSource.InstrumentedApplicationDetails, - ServiceName: &k8sSource.ReportedName, - } -} - func instrumentedApplicationToActualSource(instrumentedApp v1alpha1.InstrumentedApplication) *gqlmodel.K8sActualSource { // Map the container runtime details var containers []*gqlmodel.SourceContainerRuntimeDetails @@ -121,34 +70,14 @@ func instrumentedApplicationToActualSource(instrumentedApp v1alpha1.Instrumented }) } - // Map the options for instrumentation libraries - var instrumentationOptions []*gqlmodel.InstrumentedApplicationDetails - for _, option := range instrumentedApp.Spec.Options { - for _, libOptions := range option.InstrumentationLibraries { - var libraries []*gqlmodel.InstrumentationOption - for _, configOption := range libOptions.Options { - libraries = append(libraries, &gqlmodel.InstrumentationOption{ - OptionKey: configOption.OptionKey, - SpanKind: gqlmodel.SpanKind(configOption.SpanKind), - }) - } - - instrumentationOptions = append(instrumentationOptions, &gqlmodel.InstrumentedApplicationDetails{ - Containers: containers, - Conditions: conditions, - }) - } - } - // Return the converted K8sActualSource object return &gqlmodel.K8sActualSource{ - Namespace: instrumentedApp.Namespace, - Kind: k8sKindToGql(instrumentedApp.OwnerReferences[0].Kind), - Name: instrumentedApp.OwnerReferences[0].Name, - ServiceName: &instrumentedApp.Name, - NumberOfInstances: nil, - AutoInstrumented: instrumentedApp.Spec.Options != nil, - AutoInstrumentedDecision: "", + Namespace: instrumentedApp.Namespace, + Kind: k8sKindToGql(instrumentedApp.OwnerReferences[0].Kind), + Name: instrumentedApp.OwnerReferences[0].Name, + ServiceName: &instrumentedApp.Name, + NumberOfInstances: nil, + AutoInstrumented: instrumentedApp.Spec.Options != nil, InstrumentedApplicationDetails: &gqlmodel.InstrumentedApplicationDetails{ Containers: containers, Conditions: conditions, diff --git a/frontend/graph/schema.resolvers.go b/frontend/graph/schema.resolvers.go index b3c4b18a9..434332ccd 100644 --- a/frontend/graph/schema.resolvers.go +++ b/frontend/graph/schema.resolvers.go @@ -75,16 +75,7 @@ func (r *computePlatformResolver) K8sActualNamespaces(ctx context.Context, obj * // K8sActualSource is the resolver for the k8sActualSource field. func (r *computePlatformResolver) K8sActualSource(ctx context.Context, obj *model.ComputePlatform, name *string, namespace *string, kind *string) (*model.K8sActualSource, error) { - source, err := services.GetActualSource(ctx, *namespace, *kind, *name) - if err != nil { - return nil, err - } - if source == nil { - return nil, nil - } - k8sActualSource := k8sSourceToGql(source) - - return k8sActualSource, nil + return nil, nil } // K8sActualSources is the resolver for the k8sActualSources field. @@ -100,7 +91,7 @@ func (r *computePlatformResolver) K8sActualSources(ctx context.Context, obj *mod // Convert each instrumented application to the K8sActualSource type for _, app := range instrumentedApplications.Items { actualSource := instrumentedApplicationToActualSource(app) - + services.AddHealthyInstrumentationInstancesCondition(ctx, &app, actualSource) owner, _ := services.GetWorkload(ctx, actualSource.Namespace, string(actualSource.Kind), actualSource.Name) if owner == nil { @@ -152,14 +143,14 @@ func (r *computePlatformResolver) Actions(ctx context.Context, obj *model.Comput return nil, err } for _, action := range icaActions.Items { - specStr, err := json.Marshal(action.Spec) // Convert spec to JSON string + specStr, err := json.Marshal(action.Spec) if err != nil { return nil, err } response = append(response, &model.IcaInstanceResponse{ ID: action.Name, Type: action.Kind, - Spec: string(specStr), // Return the JSON string + Spec: string(specStr), }) } diff --git a/frontend/services/sources.go b/frontend/services/sources.go index 2a9d0992f..9549fba7a 100644 --- a/frontend/services/sources.go +++ b/frontend/services/sources.go @@ -62,42 +62,6 @@ type ThinSource struct { IaDetails *InstrumentedApplicationDetails `json:"instrumented_application_details"` } -func GetActualSource(ctx context.Context, ns string, kind string, name string) (*Source, error) { - k8sObjectName := workload.CalculateWorkloadRuntimeObjectName(name, kind) - owner, numberOfRunningInstances := GetWorkload(ctx, ns, kind, name) - if owner == nil { - return nil, fmt.Errorf("owner not found") - } - ownerAnnotations := owner.GetAnnotations() - var reportedName string - if ownerAnnotations != nil { - reportedName = ownerAnnotations[consts.OdigosReportedNameAnnotation] - } - - ts := ThinSource{ - SourceID: SourceID{ - Namespace: ns, - Kind: kind, - Name: name, - }, - NumberOfRunningInstances: numberOfRunningInstances, - } - - instrumentedApplication, err := kube.DefaultClient.OdigosClient.InstrumentedApplications(ns).Get(ctx, k8sObjectName, metav1.GetOptions{}) - if err == nil { - ts.IaDetails = k8sInstrumentedAppToThinSource(instrumentedApplication).IaDetails - err = addHealthyInstrumentationInstancesCondition(ctx, instrumentedApplication, &ts) - if err != nil { - return nil, err - } - } - - return &Source{ - ThinSource: ts, - ReportedName: reportedName, - }, nil -} - func GetWorkload(c context.Context, ns string, kind string, name string) (metav1.Object, int) { switch kind { case "Deployment": @@ -123,7 +87,7 @@ func GetWorkload(c context.Context, ns string, kind string, name string) (metav1 } } -func addHealthyInstrumentationInstancesCondition(ctx context.Context, app *v1alpha1.InstrumentedApplication, source *ThinSource) error { +func AddHealthyInstrumentationInstancesCondition(ctx context.Context, app *v1alpha1.InstrumentedApplication, source *model.K8sActualSource) error { labelSelector := fmt.Sprintf("%s=%s", consts.InstrumentedAppNameLabel, app.Name) instancesList, err := kube.DefaultClient.OdigosClient.InstrumentationInstances(app.Namespace).List(ctx, metav1.ListOptions{ LabelSelector: labelSelector, @@ -150,48 +114,23 @@ func addHealthyInstrumentationInstancesCondition(ctx context.Context, app *v1alp } } - status := metav1.ConditionTrue + status := model.ConditionStatusTrue if healthyInstances < totalInstances { - status = metav1.ConditionFalse + status = model.ConditionStatusFalse } - source.IaDetails.Conditions = append(source.IaDetails.Conditions, metav1.Condition{ + message := fmt.Sprintf("%d/%d instances are healthy", healthyInstances, totalInstances) + lastTransitionTime := Metav1TimeToString(latestStatusTime) + source.InstrumentedApplicationDetails.Conditions = append(source.InstrumentedApplicationDetails.Conditions, &model.Condition{ Type: "HealthyInstrumentationInstances", Status: status, - LastTransitionTime: latestStatusTime, - Message: fmt.Sprintf("%d/%d instances are healthy", healthyInstances, totalInstances), + LastTransitionTime: &lastTransitionTime, + Message: &message, }) return nil } -func k8sInstrumentedAppToThinSource(app *v1alpha1.InstrumentedApplication) ThinSource { - var source ThinSource - source.Name = app.OwnerReferences[0].Name - source.Kind = app.OwnerReferences[0].Kind - source.Namespace = app.Namespace - var conditions []metav1.Condition - for _, condition := range app.Status.Conditions { - conditions = append(conditions, metav1.Condition{ - Type: condition.Type, - Status: condition.Status, - Message: condition.Message, - LastTransitionTime: condition.LastTransitionTime, - }) - } - source.IaDetails = &InstrumentedApplicationDetails{ - Languages: []SourceLanguage{}, - Conditions: conditions, - } - for _, language := range app.Spec.RuntimeDetails { - source.IaDetails.Languages = append(source.IaDetails.Languages, SourceLanguage{ - ContainerName: language.ContainerName, - Language: string(language.Language), - }) - } - return source -} - func GetWorkloadsInNamespace(ctx context.Context, nsName string, instrumentationLabeled *bool) ([]model.K8sActualSource, error) { namespace, err := kube.DefaultClient.CoreV1().Namespaces().Get(ctx, nsName, metav1.GetOptions{}) diff --git a/frontend/services/utils.go b/frontend/services/utils.go index bf0b7db2c..71218b665 100644 --- a/frontend/services/utils.go +++ b/frontend/services/utils.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "strings" + "time" "github.com/odigos-io/odigos/common" "github.com/odigos-io/odigos/frontend/graph/model" @@ -78,3 +79,10 @@ func DerefString(s *string) string { func StringPtr(s string) *string { return &s } + +func Metav1TimeToString(latestStatusTime metav1.Time) string { + if latestStatusTime.IsZero() { + return "" + } + return latestStatusTime.Time.Format(time.RFC3339) +}