-
Notifications
You must be signed in to change notification settings - Fork 528
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
feat: start hiding beats monitoring behind otel abstraction #15360
base: main
Are you sure you want to change the base?
Conversation
This pull request does not have a backport label. Could you fix it @kruskall? 🙏
|
|
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.
Thanks for splitting this out, it's a lot more consumable!
Mostly looks good, just a few comments
- Only adapt go-docappender metrics when using the Elasticsearch output - Increment processed events properly. - Create object hierarchy rather than dotted metric names so _source remains backwards compatible. Revert system test changes. - Report elasticsearch.indexers.active; use correct names for go-docappender indexer creation/destruction OTel metrics. ... and add a unit test for all of that
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, we'll need to make sure stack monitoring & self-instrumentation are manually tested I think.
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.
for _, dp := range data.DataPoints { | ||
status, ok := dp.Attributes.Value(attribute.Key("status")) | ||
if !ok { | ||
continue |
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.
Should we log a warning here? Afaics this wouldn't be expected.
Motivation/summary
Split from #15094
part 1
Checklist
For functional changes, consider:
How to test these changes
Related issues