-
Notifications
You must be signed in to change notification settings - Fork 179
Move the “Providers" docs out of the standalone Elastic Agent docs #3919
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
base: main
Are you sure you want to change the base?
Conversation
|
@blakerouse I’ve updated the docs based on your comment in the related issue, so I’d appreciate it if you could have a look at the changes I’m making in the |
karenzone
left a comment
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.
Left minor suggestion inline. Please ping me if you'd like a review after you and the developers get content nailed down.
reference/fleet/providers.md
Outdated
| ### Standalone {{agent}} | ||
|
|
||
| On standalone {{agent}}, providers can be configured through the `providers` key in the `elastic-agent.yml` configuration file. You can enable, disable, and configure provider settings as needed. For more details, refer to [Provider configuration](#provider_configuration). | ||
|
|
||
| ### {{fleet}}-managed {{agent}} |
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.
This is nice work, @vishaangelova.
A couple of things while we wait for reviews from Dev...
Consider making "Providers" part of the headings--like "Configure providers for standalone agent and "Configure providers for Fleet-managed agent." We get better SEO results when key words are adjacent. If one of these topics comes up in search results, the user might be more likely to click if the heading pro to answer their question.
Also some new headings need anchors defined.
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.
You are right… About the titles - would it not be confusing to have “Configure providers…” here as the next second-level heading is “Provider configuration”? Maybe we can rephrase to "Using providers on standalone Elastic Agent” and "Using providers on Fleet-managed Elastic Agent”? WDYT?
About the anchors - I’ve seen people add new content and headings without anchors, probably relying on the heanding’s automatic anchor link for refs - but I guess that may not be a good practice in case the titles change.
In this particular case, I though it would make more sense to link to the second-level heading (## Provider usage by deployment model) if it was ever needed, but that’s an assumption I probably shouldn’t make. I’ll add explicit anchor links.
blakerouse
left a comment
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.
Overall this looks good. I provided an early comment that I think would be good to provide that context sooner, but it was eventual address if you read the whole document. I don't know much about structure, but overall the document ultimately says the point I was making in my first comment.
|
@blakerouse I’ve updated the doc based on your comments in 3e77779 - could you have a look? Thanks! |
e5f8e9b to
93df482
Compare
bmorelli25
left a comment
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.
Two final comments. Nice job 🚀
reference/fleet/providers.md
Outdated
|
|
||
| ### Dynamic providers [dynamic-providers] | ||
|
|
||
| Dynamic providers supply an array of multiple key-value mappings. Each key-value mapping is combined with the previous context provider’s key and value mapping which provides a new unique mapping that is used to generate a configuration. |
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 know this sentence existed already, but it's really confusing for me. Is this accurate?
Dynamic providers supply multiple key-value mappings. Each mapping represents a separate “item” or resource. {{agent}} merges each mapping with context providers to create a separate configuration for each item.
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.
Hmm, I used Cursor to search for an answer, and I made it check the code in the elastic-agent repo and compare with that. Here’s what it tells me:
“Dynamic providers supply an array of multiple key-value mappings”: This is accurate.
The bullet descriptions are accurate.
What is imprecise/misleading:
“combined with the previous context provider’s key and value mapping” is not how the code conceptualizes it:
- Internally, there is a global context built from all context providers, and a set of dynamic mappings from each dynamic provider (DynamicProviderComm.AddOrUpdate).
- When rendering configurations, the composable controller matches each dynamic mapping to config templates (often using conditions) and creates a separate rendered config per matching mapping, substituting variables like ${docker.container.id} or ${kubernetes.pod.ip}.
- There is no special notion of “the previous” context provider; mappings are merged into the overall state with priorities.
So, the high‑level idea (dynamic providers drive multiple config instances from many items) is true, but the current wording about “previous context provider” is inaccurate and could confuse users.
It also confirmed your suggestion as accurate but I have a slightly more detailed description based on Cursor’s findings (see 016e30e).
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.
Nice!
Vale Linting ResultsSummary: 9 suggestions found 💡 Suggestions (9)
|
4eed019 to
016e30e
Compare
|
@blakerouse since your last review, I’ve made some (mainly) structural changes in 93df482, for which I have @bmorelli25’s approval, but I’d really appreciate it if you can have a look at the changes in the last two commits, so I don’t make any incorrect statements: afbfe8f and 016e30e (they attempt to improve the sections on context and dynamic providers). Thank you! |
This PR moves the “Providers" docs out of the standalone Elastic Agent docs, so they are now at the level of the Elastic Agent processors.
It also:
providers.mdwith more details to make the content more accurate (e.g., for the context and dynamic providers), and includes information on using providers on Fleet-managed Elastic Agent (vs. standalone).applies_totag to all provider docs.Closes #2167
———
An AI assistant (Cursor) was used in this PR to clarify and improve the content in the
providers.mddoc, by checking against the code base for correct/incorrect statements.———
Previews
reference/fleet/agent-provider.md
reference/fleet/docker-provider.md
reference/fleet/env-provider.md
reference/fleet/filesource-provider.md
reference/fleet/host-provider.md
reference/fleet/kubernetes-provider.md
reference/fleet/kubernetes_leaderelection-provider.md
reference/fleet/kubernetes_secrets-provider.md
reference/fleet/local-dynamic-provider.md
reference/fleet/local-provider.md
reference/fleet/providers.md