Skip to content
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 ingest_pipeline data streams and dashboards to Elasticsearch package #4597

Merged
merged 21 commits into from Feb 24, 2023

Conversation

joshdover
Copy link
Member

@joshdover joshdover commented Nov 8, 2022

What does this PR do?

Part of elastic/kibana#41936

This adds support to the existing Elasticsearch package for the new ingest_pipeline metricset added in Metricbeat in 8.7.0 in: elastic/beats#34012.

This started out as an httpjson input and eventually was moved to Metricbeat to improve it's availability to more customers. See the main issue for more discussion.

Still TODO:

  • Determine a path forward on node-level granularity
    • Due to this limitation in Lens, we cannot create a line graph that shows the stats broken down by pipeline, summed across all nodes in the cluster: [Lens] collapse by only one field for multi-field top values kibana#146733
    • We can create tables that do this however
    • We either need to accept this limitation for now, wait until the Kibana issue is resolved, or remove the node-level granularity from the stats and sum the values across all nodes before sending to ES
    • For now we're going to accept this limitation and keep the node-level granularity in the metrics. I'll work with the Kibana team to prioritize the viz fix.
  • Add cluster name / ID to each document
    • cluster ID isn't available in node stats API so just using name for now
  • If we keep node-level granularity, add node name and role to each doc
  • Update the index pattern used to be able to reference metrics-*,.monitoring-*,metricbeat-* so it's generally useful for all collection methods
  • Add processor-level metrics, likely as a separate data stream
    • Challenges: this may create a very large volume of data that isn't likely to be used on a day-to-day basis. Need to determine a sampling or opt-in strategy for aiding optimization workflows separately from high-level monitoring
    • Decision: a new metricbeat configuration was added to sample process-level metrics. The default is 25%.
  • Implement collection and processing in metricbeat
  • Update dashboard copy
    • Reword "misbehaving processors" copy on pipeline detail dashboard to be more clear
    • Add unit labels to axis where appropriate
    • Double check consistent labels are used across visualizations, eg. "events per second" vs. "documents processed" etc.
  • Create and prioritize user stories and questions we want to be sure the dashboard can answer
    • Add drilldowns & links into Ingest Pipeline editor and Stack Monitoring stats where appropriate
  • Add link to ingest pipeline dashboard from Stack Monitoring kibana#149721
  • Determine how to handle the drilldowns to the pipeline editor when monitoring data is in a separate cluster
    • The "Edit pipeline" button doesn't work if the monitoring data is in a separate cluster from the one being monitored (which is typical).
    • See if we can provide a way to insert the Kibana URL of the cluster being monitored into the drilldown link to provide cross-linking
    • Otherwise, we may want to remove this for now.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Related issues

Screenshots

image

image

@joshdover joshdover requested a review from a team as a code owner November 8, 2022 22:01
@elasticmachine
Copy link

elasticmachine commented Nov 8, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-02-24T10:30:38.526+0000

  • Duration: 30 min 22 sec

Test stats 🧪

Test Results
Failed 0
Passed 59
Skipped 0
Total 59

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Nov 8, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (5/5) 💚
Files 100.0% (9/9) 💚
Classes 100.0% (9/9) 💚
Methods 87.5% (98/112) 👎 -8.402
Lines 91.98% (562/611) 👍 3.162
Conditionals 100.0% (0/0) 💚

@joshdover joshdover marked this pull request as draft November 9, 2022 11:01
Copy link
Contributor

@miltonhultgren miltonhultgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like what I imagine the future would be.

As we add new signals we'd expose them as new Integrations with their own datasets. It doesn't truly matter if they're using OpenTelemetry (although that will likely be the goal for anything that is metrics to replace the existing datasets).

The only thing we'd need to do is re-arrange the order the Integrations show up in and change the labels of the Stack Monitoring ones to highlight that these are legacy sources used to power the Stack Monitoring UI only.

Screenshot 2022-11-23 at 17 20 32

packages/elasticsearch/data_stream/ingest/manifest.yml Outdated Show resolved Hide resolved
packages/elasticsearch/data_stream/ingest/manifest.yml Outdated Show resolved Hide resolved
Comment on lines 4 to 8
{{#if username}}
auth.basic:
user: {{username}}
password: {{password}}
{{/if}}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we support API keys too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, I noticed we don't do that for the other data sets, we're tracking that here #4666

@philippkahr
Copy link
Contributor

Love the design of this, I do have a few ideas for the dashboard:

We already have the cpu usage in % in the stack monitoring data, we could add this as a line view, normalised to 100% usage for all nodes?

image

I think a general chart that shows how much documents are ingested, vs how much are passing through an ingest pipeline? So I also am not mislead by seeing a drop.

@joshdover
Copy link
Member Author

@philippkahr How's this look? I worry a bit that this makes the top chart a bit busy, but it may be worth it. We could average over all nodes instead of breaking down by node, but I don't think that's very useful.

image

@joshdover
Copy link
Member Author

This has been updated with several new fields:

  • elasticsearch.node.name
  • elasticsearch.node.roles
  • elasticsearch.cluster.name

We also now have drilldown links from the table views that will go either the Stack Monitoring page for node stats or the ingest pipeline editor.

@joshdover
Copy link
Member Author

@jsoriano @hop-dev I'd like to have this dashboard reference a data view that covers both Agent data streams, Metricbeat indices, and .monitoring-* indices. Do we have any other integrations that do something similar (I couldn't find any)? Should I just add/export a data view / index pattern object directly to this package to accomplish this? Is that supported by Fleet?

@hop-dev
Copy link
Contributor

hop-dev commented Dec 8, 2022

@joshdover Cloud security posture define their own index patterns for an example:
https://github.com/elastic/integrations/blob/d5a04f98392ee1441c96314545161bbab8015381/packages/cloud_security_posture/kibana/index_pattern/cloud_security_posture-303eea10-c475-11ec-af18-c5b9b437dbbe.json

@joshdover joshdover changed the title [POC] Add ingest pipeline data streams to es package Add ingest pipeline data streams to es package Dec 14, 2022
@joshdover
Copy link
Member Author

This is ready for review. It now uses the new ingest_pipeline metricset shipping in 8.7.0.

Note this bumps the minimum required version of the Stack to 8.7.0.

@joshdover joshdover marked this pull request as ready for review January 19, 2023 19:39
@joshdover
Copy link
Member Author

Currently failing right now with this error:

[elastic_agent][error] Unit state changed elasticsearch/metrics-default (STARTING->FAILED): [failed to reloading inputs: 1 error: Error creating runner from config: 1 error: metricset 'elasticsearch/ingest_pipeline' not found]

Traced through the code a bit and it's not obvious why this is happening. I suspect a stale build of Beats right now

@joshdover
Copy link
Member Author

Confirmed that this is working on the latest 8.7.0-SNAPSHOT of Agent. This is ready for review @miltonhultgren and @klacabane

@miltonhultgren
Copy link
Contributor

Looking at this now 👍🏼

Copy link
Contributor

@miltonhultgren miltonhultgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

The one thing I'm not sure about is if there is a need to add more for the tests to work? I haven't look into Integration system testing yet.

packages/elasticsearch/changelog.yml Outdated Show resolved Hide resolved
@miltonhultgren miltonhultgren removed the request for review from klacabane February 2, 2023 19:46
@joshdover
Copy link
Member Author

This is ready to be merged, however we will wait to merge it for a few more weeks. The reason being that we want some extra bake time on the recently GA'd ES package and want to be able to easily releases fixes to in Stack 8.5.0 before we introduce this PR which will only target 8.7.0+

@joshdover
Copy link
Member Author

@klacabane @miltonhultgren How do you all feel about merging/releasing this tomorrow, as version 1.3.0 (GA). This would unblock testing of the new feature end-to-end on Cloud.

@klacabane
Copy link
Contributor

That works. let's see if we can get #5353 merged as 1.3.0 first, then we can release this as 1.4.0 with the new kibana requirement

@joshdover
Copy link
Member Author

Updated the version to 1.4.0

@joshdover
Copy link
Member Author

Actually going to release this as 1.4.0-beta1 and then will promote to GA as a separate PR/release

@joshdover joshdover merged commit 2abe6f6 into elastic:main Feb 24, 2023
@joshdover joshdover deleted the es-ingest branch February 24, 2023 11:31
@joshdover joshdover mentioned this pull request Feb 27, 2023
5 tasks
agithomas pushed a commit to agithomas/integrations that referenced this pull request Mar 20, 2023
agithomas pushed a commit to agithomas/integrations that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants