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

Generalize instrumentation manager #1980

Merged
merged 13 commits into from
Dec 15, 2024

Conversation

RonFed
Copy link
Collaborator

@RonFed RonFed commented Dec 12, 2024

A follow up to #1776:

  • Make the instrumentation manager generic, and move it to a new module.
  • Add a k8s-oriented implementation of the manager's different interfaces under odiglet/pkg/ebpf

instrumentation/manager.go Outdated Show resolved Hide resolved
@RonFed RonFed requested review from blumamir and edeNFed December 13, 2024 14:13
@RonFed RonFed requested review from damemi and tamirdavid1 December 15, 2024 07:40
instrumentation/manager.go Outdated Show resolved Hide resolved
instrumentation/manager.go Outdated Show resolved Hide resolved
instrumentation/manager.go Outdated Show resolved Hide resolved
// The manager will apply the configuration to all instrumentations that match the config group.
type ConfigUpdate[configGroup ConfigGroup] map[configGroup]Config

type instrumentationDetails[details Details, configGroup ConfigGroup] struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we already have Details, and now instrumentationDetails with a property details inside it, can we have better names so it's less confusing?

This creates code like details.details

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed Details to ProcessGroup and added comments explaining the meaning behind it

@RonFed RonFed merged commit 240c378 into odigos-io:main Dec 15, 2024
31 checks passed
@RonFed RonFed deleted the instrumentation_reportee branch December 15, 2024 15:47
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