[GEN-1738] Add service name to instrumentation config #1830
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ntainer Task 149 sources container
…play-source-manage-list
…urces-card Task 107 overview sources card
…rce-manage-list Task 142 display source manage list
…rce-btn Task 143 add new source btn
…sources-connection Task 145 handle new sources connection
…-service-name-to-instrumentation-config
blumamir
reviewed
Nov 24, 2024
instrumentor/controllers/instrumentationconfig/workloads_reconciler.go
Outdated
Show resolved
Hide resolved
instrumentor/controllers/instrumentationconfig/workloads_reconciler.go
Outdated
Show resolved
Hide resolved
instrumentor/controllers/instrumentationconfig/workloads_reconciler.go
Outdated
Show resolved
Hide resolved
instrumentor/controllers/instrumentationconfig/workloads_reconciler.go
Outdated
Show resolved
Hide resolved
instrumentor/controllers/instrumentationconfig/instrumentedapplication_controller.go
Outdated
Show resolved
Hide resolved
… github.com:alonkeyval/odigos into gen-1738-add-service-name-to-instrumentation-config
RonFed
reviewed
Nov 24, 2024
instrumentor/controllers/instrumentationconfig/workloads_reconciler.go
Outdated
Show resolved
Hide resolved
blumamir
reviewed
Nov 25, 2024
instrumentor/controllers/instrumentationconfig/workloads_controllers.go
Outdated
Show resolved
Hide resolved
instrumentor/controllers/instrumentationconfig/workloads_controllers.go
Outdated
Show resolved
Hide resolved
… github.com:alonkeyval/odigos into gen-1738-add-service-name-to-instrumentation-config
RonFed
reviewed
Nov 25, 2024
Comment on lines
+193
to
+201
| func ExtractServiceNameFromAnnotations(annotations map[string]string, defaultName string) string { | ||
| if annotations == nil { | ||
| return defaultName | ||
| } | ||
| if reportedName, exists := annotations[consts.OdigosReportedNameAnnotation]; exists && reportedName != "" { | ||
| return reportedName | ||
| } | ||
| return defaultName | ||
| } |
There was a problem hiding this comment.
I remember @blumamir had a code for generating the service name based on the container name as well - to avoid conflicting service names. Do we need to handle this here?
| return defaultName | ||
| } | ||
| if reportedName, exists := annotations[consts.OdigosReportedNameAnnotation]; exists && reportedName != "" { | ||
| return reportedName |
There was a problem hiding this comment.
Do we need to perform some validation on this user-provided value?
Also, the fallbacks defined by OTel: https://opentelemetry.io/docs/specs/semconv/resource/#service
We can say that we'll handle this in the lower layers reading the service name from the config - I'm not sure what it the best approach.
blumamir
reviewed
Nov 26, 2024
|
|
||
| } | ||
|
|
||
| func createOrUpdateInstrumentationConfig(ctx context.Context, k8sClient client.Client, instConfigName, namespace string, serviceName string) (reconcile.Result, error) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces changes to support the identification and management of service names in telemetry data. The most important changes include the addition of the
serviceNameattribute in various configurations and the implementation of logic to resolve and update service names based on workload annotations.Enhancements to Telemetry Data Management:
api/config/crd/bases/odigos.io_instrumentationconfigs.yaml: AddedserviceNameattribute to identify the name of the service generating telemetry data.api/odigos/v1alpha1/instrumentationconfig_types.go: IntroducedServiceNamefield inInstrumentationConfigSpecto store the service name.Updates to Controllers:
instrumentor/controllers/instrumentationconfig/common.go: ModifiedupdateInstrumentationConfigForWorkloadto acceptserviceNameand addedresolveServiceNamefunction to determine the service name based on workload kind. [1] [2] [3]instrumentor/controllers/instrumentationconfig/instrumentationrule_controller.go: Integrated service name resolution in theReconcilemethod and updated calls toupdateInstrumentationConfigForWorkload. [1] [2]instrumentor/controllers/instrumentationconfig/instrumentedapplication_controller.go: Added logic to resolve and use the service name in theReconcilemethod.Testing Updates:
instrumentor/controllers/instrumentationconfig/common_test.go: Updated test cases to passserviceNametoupdateInstrumentationConfigForWorkload. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]New Workload Reconciler:
instrumentor/controllers/instrumentationconfig/manager.go: Added predicates and watchers for deployment, statefulset, and daemonset changes to handle service name updates. [1] [2] [3]instrumentor/controllers/instrumentationconfig/workloads_reconciler.go: ImplementedWorkloadsReconcilerto manageInstrumentationConfigupdates based on workload annotations.