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

Remove default SDKs and supported SDKs from Odigos config #1626

Merged
merged 4 commits into from
Oct 27, 2024

Conversation

RonFed
Copy link
Collaborator

@RonFed RonFed commented Oct 26, 2024

Removing the default SDKs from odigos config.
The updated way of changing instrumentation SDK is with rules and profiles.

This change includes:

  • Remove the setting of the SDKs by the CLI.
  • Add to instrumentor: checking the tier and setting the default SDKs map once.
  • Add role to instrumentor to get secrets in odigos ns.
  • Remove odigosconfig role for instrumentor.
  • Adjust envtest for env overwriter - instead of reading writing to mock of odigosconfig - mock the default SDK for the new code and simulate a changing SDK by mocking instrumentation rule.

@RonFed RonFed changed the title Remove sdks from config Remove default SDKs and supported SDKs from Odigos config Oct 26, 2024
Comment on lines -48 to -49
SupportedSDKs map[common.ProgrammingLanguage][]common.OtelSdk `json:"supportedSDKs,omitempty"`
DefaultSDKs map[common.ProgrammingLanguage]common.OtelSdk `json:"defaultSDKs,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

},
APIGroups: []string{""},
Resources: []string{
"secrets",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good for now.
In the future, we might want to consider storing the tier in the odigos-deployment configmap, so we can read the tier easily without needing permissions to secrets. We can also use envFrom to automatically have these as env variables in the code

@RonFed RonFed merged commit 0544864 into odigos-io:main Oct 27, 2024
26 checks passed
@RonFed RonFed deleted the remove_sdks_from_config branch October 27, 2024 08:33
RonFed pushed a commit that referenced this pull request Nov 9, 2024
This controller was relevant when the defaultSdks were read from the
odigos config.
Once it's removed in #1626 , now the instrumentor read the default sdks
based on the tier on boot, and no longer consumes it from this config
map.

Notice that any change in tier will not be picked up automatically to
update the device names (was not supported before this PR).
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.

2 participants