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.

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

Add event filter for instrumentation-device CG reconciler #1713

Merged
merged 3 commits into from
Nov 9, 2024

Conversation

RonFed
Copy link
Contributor

@RonFed RonFed commented Nov 8, 2024

During the un-instrumentation of a large number of sourcesreconcileSingleWorkload may be called concurrently by the collectors group reconciler and the instrumented-application reconciler.

This race condition can cause the instrumentation device to be added right after its removal as demonstrated in the following log:

2024-11-08T12:18:23Z	INFO	removed instrumentation device from workload	{"controller": "instrumentationdevice-instrumentedapplication", "controllerGroup": "odigos.io", "controllerKind": "InstrumentedApplication", "InstrumentedApplication": {"name":"deployment-coupon","namespace":"simple-demo18"}, "namespace": "simple-demo18", "name": "deployment-coupon", "reconcileID": "a50d0c85-fd07-4896-84a3-817472fc3953", "namespace": "simple-demo18", "kind": "&TypeMeta{Kind:Deployment,APIVersion:apps/v1,}", "name": "coupon", "reason": "NoRuntimeDetails"}
2024-11-08T12:18:23Z	DEBUG	instrumented application deleted	{"controller": "deleteinstrumentedapplication-deployment", "controllerGroup": "apps", "controllerKind": "Deployment", "Deployment": {"name":"coupon","namespace":"simple-demo18"}, "namespace": "simple-demo18", "name": "coupon", "reconcileID": "08560386-4960-4ace-aaae-343fd6fb4cc2", "namespace": "simple-demo18", "name": "coupon", "kind": "Deployment"}
2024-11-08T12:18:23Z	INFO	added instrumentation device to workload	{"controller": "instrumentationdevice-collectorsgroup", "controllerGroup": "odigos.io", "controllerKind": "CollectorsGroup", "CollectorsGroup": {"name":"odigos-data-collection","namespace":"odigos-system"}, "namespace": "odigos-system", "name": "odigos-data-collection", "reconcileID": "52989ae7-2686-4e1c-99cb-23b3252535ad", "name": "coupon", "namespace": "simple-demo18"}

To address this, an event filter is added to the CG reconciler, which only forward either

  1. Create events which have CG ready.
  2. Update events where the CG became ready.

In addition, the reconciler is updated to get a fresh copy from the cache of each instrumented app it handles before it reconciles it. This should reduce the chance of handling an old instrumented application and reconciling workloads unnecessarily.

@RonFed RonFed merged commit 7300b2a into odigos-io:main Nov 9, 2024
26 checks passed
@RonFed RonFed deleted the cg_reconcile_predicate branch November 9, 2024 11:03
RonFed added a commit that referenced this pull request Nov 10, 2024
This PR addresses 2 main issues in the config map reconciler in the lang
detection package inside the instrumentor.
The only goal of this reconciler is to trigger runtime detection in case
of a change in the ignored containers field in Odigos config.

1. Change the event filter predicate to only pass data updates of the
config map - and only for the Odigos config cm.
Adding a util predicate for the config map data change one.
3. Fix possible stale data being read from the workloads lists by
re-getting from the cache before deciding. similar to #1713 and #1716

note: the reconcile logic can in the OdigosConfig reconciler can be
improved by iterating over the instrumentationconfigs and marking them
to perform runtime detection - instead of iterating over the workloads
which is what is done today. This is left out of this PR since the
runtime detection signaling is constantly being refactored, and for
making this PR smaller and easier to review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants