Revamp config loading and support datadog.yaml #359
Conversation
Properly distinguish the three sources of config (ini, yaml, env) and merge them in a clear order. Make it clear that -ddconfig is a deprecated option.
LotharSee
changed the title
agent/main.go
Outdated
| // deprecated Agent 5 trace-agent.ini config | ||
| var legacyConf *config.File | ||
|
|
||
| if filepath.Ext(opts.configFile) == ".conf" { |
There was a problem hiding this comment.
To make the precedence clear, I would probably log a line regarding what config got loaded.
There was a problem hiding this comment.
Good one, updated with more logging.
| } | ||
|
|
||
| // [trace.writer.*] sections | ||
| // undocumented, should they stay all internal? |
There was a problem hiding this comment.
Yeah we can keep most of them internal/hidden. Only one that has to be documented imo is the trace.writer.traces.queue_max_bytes and explain the tradeoff involved: more memory => more traces saved in case of problems but more system impact.
There was a problem hiding this comment.
yup, keeping them here for now for our testing, will remove it in later versions.
| return c | ||
| } | ||
|
|
||
| // TODO: maybe this is too many options exposed? |
There was a problem hiding this comment.
Right. Kept them around until we did some experimentation with the new retry scenario. Aaditya's experiments seem to suggest it's working nicely so we could remove them. Although as long as we keep them internal, they don't really affect usability and will allow us to provide an easy hotfix if we ever detect a problem here.
There was a problem hiding this comment.
yup, keeping them here for now for our testing, will remove it in later versions.
| flag.StringVar(&opts.configFile, "config", "/etc/datadog/trace-agent.ini", "Trace agent ini config file.") | ||
| // TODO: load from the .yaml automatically if there | ||
| flag.StringVar(&opts.configFile, "config", "/etc/dd-agent/datadog.conf", "Datadog Agent config file location") | ||
| flag.StringVar(&opts.legacyConfigFile, "ddconfig", "/etc/dd-agent/trace-agent.ini", "Deprecated extra configuration option.") |
There was a problem hiding this comment.
This requires changes here:
Line 148 in 9bd73a0
- Devenv
- Devops
Built on top of rebase of #357 this PRs supersedes it.
Exhaustive changes:
From #357:
apm_configAdded on top:
APIPayloadBufferMaxSizeagent/trace-agent.iniand cleanconfig/README.md. Now we should only refer to the A5 and A6 example configs.-ddconfigmaking-configthe only supported way of configuring the Agent.-ddconfigis kept for a bit before getting fully removed.