-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 APM trace configuration to V2AgentManager #40030
Changes from 5 commits
674b47c
d477c9f
b14aa0c
2da128b
1e194cb
9cc1b8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,10 +102,14 @@ type BeatV2Manager struct { | |
// set with the last applied input configs | ||
lastInputCfgs map[string]*proto.UnitExpectedConfig | ||
|
||
// set with the last applied APM config | ||
lastAPMCfg *proto.APMConfig | ||
|
||
// used for the debug callback to report as-running config | ||
lastBeatOutputCfg *reload.ConfigWithMeta | ||
lastBeatInputCfgs []*reload.ConfigWithMeta | ||
lastBeatFeaturesCfg *conf.C | ||
lastBeatAPMCfg *reload.ConfigWithMeta | ||
|
||
// changeDebounce is the debounce time for a configuration change | ||
changeDebounce time.Duration | ||
|
@@ -669,6 +673,10 @@ func (cm *BeatV2Manager) reload(units map[unitKey]*agentUnit) { | |
cm.logger.Errorw("setting output state", "error", err) | ||
} | ||
|
||
// reload APM tracing configuration | ||
// all error handling is handled inside of reloadAPM | ||
cm.reloadAPM(outputUnit) | ||
|
||
// compute the input configuration | ||
// | ||
// in v2 only a single input type will be started per component, so we don't need to | ||
|
@@ -865,6 +873,65 @@ func (cm *BeatV2Manager) reloadInputs(inputUnits []*agentUnit) error { | |
return nil | ||
} | ||
|
||
// reloadAPM reload APM tracing, it returns a bool and an error. | ||
// The bool, if set, indicates that the output reload requires an restart, | ||
// in that case the error is always `nil`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment seems out of date? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copy-and-paste error, will fix. |
||
// | ||
// In any other case, the bool is always false and the error will be non nil | ||
// if any error has occurred. | ||
func (cm *BeatV2Manager) reloadAPM(unit *agentUnit) { | ||
// Assuming that the output reloadable isn't a list, see createBeater() in cmd/instance/beat.go | ||
apm := cm.registry.GetReloadableAPM() | ||
if apm == nil { | ||
// no APM reloadable, nothing to do | ||
cm.logger.Debug("Unable to reload APM tracing; no APM reloadable registered") | ||
return | ||
} | ||
|
||
var apmConfig *proto.APMConfig | ||
if unit != nil { | ||
expected := unit.Expected() | ||
if expected.APMConfig != nil { | ||
apmConfig = expected.APMConfig | ||
} | ||
} | ||
if apmConfig == nil { | ||
// APM tracing is being stopped | ||
cm.logger.Debug("Stopping APM tracing") | ||
err := apm.Reload(nil) | ||
if err != nil { | ||
cm.logger.Errorf("Error stopping APM tracing: %s", err) | ||
return | ||
} | ||
cm.lastAPMCfg = nil | ||
cm.lastBeatAPMCfg = nil | ||
cm.logger.Debug("Stopped APM tracing") | ||
return | ||
} | ||
|
||
if cm.lastAPMCfg != nil && gproto.Equal(cm.lastAPMCfg, apmConfig) { | ||
// configuration for the APM tracing did not change; do nothing | ||
cm.logger.Debug("Skipped reloading APM tracing; configuration didn't change") | ||
return | ||
} | ||
|
||
uconfig, err := conf.NewConfigFrom(apmConfig) | ||
if err != nil { | ||
cm.logger.Errorf("Failed to create uconfig from APM configuration: %s", err) | ||
return | ||
} | ||
reloadConfig := &reload.ConfigWithMeta{Config: uconfig} | ||
cm.logger.Debug("Reloading APM tracing") | ||
err = apm.Reload(reloadConfig) | ||
if err != nil { | ||
cm.logger.Debugf("Error reloading APM tracing: %s", err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: shouldn't this be logged as an error, similar to line 918? |
||
return | ||
} | ||
cm.lastAPMCfg = apmConfig | ||
cm.lastBeatAPMCfg = reloadConfig | ||
cm.logger.Debugf("Reloaded APM tracing") | ||
} | ||
|
||
// this function is registered as a debug hook | ||
// it prints the last known configuration generated by the beat | ||
func (cm *BeatV2Manager) handleDebugYaml() []byte { | ||
|
@@ -899,17 +966,28 @@ func (cm *BeatV2Manager) handleDebugYaml() []byte { | |
} | ||
} | ||
|
||
// generate APM | ||
var apmCfg map[string]interface{} | ||
if cm.lastBeatAPMCfg != nil { | ||
if err := cm.lastBeatAPMCfg.Config.Unpack(&apmCfg); err != nil { | ||
cm.logger.Errorf("error unpacking APM tracing config for debug callback: %s", err) | ||
return nil | ||
} | ||
} | ||
|
||
// combine all of the above in a somewhat coherent way | ||
// This isn't perfect, but generating a config that can actually be fed back into the beat | ||
// would require | ||
beatCfg := struct { | ||
Inputs []map[string]interface{} | ||
Outputs map[string]interface{} | ||
Features map[string]interface{} | ||
APM map[string]interface{} | ||
}{ | ||
Inputs: inputList, | ||
Outputs: outputCfg, | ||
Features: featuresCfg, | ||
APM: apmCfg, | ||
} | ||
|
||
data, err := yaml.Marshal(beatCfg) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both
reloadInputs
andreloadOutput
return an error, why doesreloadAPM
not?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because in no case, do we want APM trace configuration to cause the beat to fail. I don't think we want that, so I handle all of that logic internally inside of
reloadAPM
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the docstring on
reloadAPM
to provide this reasoning.