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

fix: start lang detection cm reconciler #1722

Merged
merged 4 commits into from
Nov 10, 2024

Conversation

RonFed
Copy link
Contributor

@RonFed RonFed commented 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.
  2. Fix possible stale data being read from the workloads lists by re-getting from the cache before deciding. similar to Add event filter for instrumentation-device CG reconciler #1713 and fix: ns reconcile race condition on delete crds #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.

@RonFed RonFed force-pushed the cm_langdetection_reconcile_fix branch from 5911c41 to d9dd276 Compare November 10, 2024 13:55
@RonFed RonFed merged commit 8684bef into odigos-io:main Nov 10, 2024
26 checks passed
@RonFed RonFed deleted the cm_langdetection_reconcile_fix branch November 10, 2024 14:19
blumamir pushed a commit that referenced this pull request Nov 10, 2024
Following #1722, use the `ConfigMapDataChangedPredicate` in conjunction
with `OdigosConfigMapPredicate` to only handle update events from our
config map in autoscaler.
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