-
Notifications
You must be signed in to change notification settings - Fork 695
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 Elastic Agent CRD (standalone mode) #4010
Conversation
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 did a very quick first pass. Looks good and works at least the default setup you provided. I want to find more time to look more closely. The only thing the stuck out a bit is the setup for the tracing where I think we need to streamline the API a bit and separate concerns between the generic log package and reconciliation a bit more maybe.
Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>
Now logs have span.id injected correctly: 2020-12-10T12:55:08.729+0100 DEBUG agent-controller test log Reconcile {"trace.id": "56a3e12652c3111375da9c9859cf4e37", "transaction.id": "56a3e12652c31113", ...} 2020-12-10T12:55:08.729+0100 DEBUG agent-controller test log doReconcile {"trace.id": "56a3e12652c3111375da9c9859cf4e37", "transaction.id": "56a3e12652c31113", "span.id": "7f516431647f01dd", ...} 2020-12-10T12:55:08.729+0100 DEBUG agent-controller test log internalReconcile {"trace.id": "56a3e12652c3111375da9c9859cf4e37", "transaction.id": "56a3e12652c31113", "span.id": "e31ac953fbd79bf7", ...} 2020-12-10T12:55:08.739+0100 DEBUG agent-controller test log reconcilePodVehicle {"trace.id": "56a3e12652c3111375da9c9859cf4e37", "transaction.id": "56a3e12652c31113", "span.id": "3d76405e4e69b6f2", ...}
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.
LGTM. I am a bit torn about elasticsearchRefs
. If we support only one sink at the moment, maybe we should stick with elasticsearchRef
because the CRD is still in alpha and can always be changed later. On the other hand, I can see that it'd be a painful migration to do.
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.
LGTM I think I am in favour of merging. Nice work! I have not tested the tracing bits. Implementationwise it looks indeed very similar to our Beats implementation and I wonder if we can merge some of the code once we have the full picture of what Agent will look like.
run full pr build |
In this PR
Added:
Additional notes
Try it out yourself!
Click to see Elasticsearch, Kibana, Elastic Agent manifest
Policies
tab go toActions
(three dots) >Add agent
>Run standalone
outputs
and use the config as the value ofconfig
in Agent specTesting
Logging and APM
As per previous discussions, our next CRD controller should pass tracing context around to allow log correlation. It turned out there is not that much of logging in the Agent controller, but the facilities were added (based on @charith-elastic work) and can be reused in the future. The logic I followed is that every function that returns
reconciler.Results
will have a matching APMSpan
and will capture errors, before bubbling them up. This way errors visible in APM provide more context. The changes in Agent controller compared to other controllers manifest as: