[Stack monitoring] logstash - read from correct aggregation #122443

klacabane · 2022-01-06T16:14:28Z

Summary

In logstash paths that need to query the pipelines nested property we have to set two aggregations, one for each of legacy and metricbeat properties (see context).
This change fixes a bug where we would exclusively read the legacy aggregation and ignore the metricbeat one, failing to compute the processing time of the vertices when the latter contains the data.

Testing

Added legacy and metricbeat functional test for the pipeline viewer. The legacy archive already included the necessary documents but I had to port one logstash_state document to the metricbeat archive (see document details)

klacabane · 2022-01-10T16:58:56Z

@elasticmachine merge upstream

klacabane · 2022-01-10T20:12:26Z

@elasticmachine merge upstream

kibanamachine · 2022-01-10T20:12:38Z

expected head sha didn’t match current head ref.

elasticmachine · 2022-01-11T17:25:04Z

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

matschaffer · 2022-01-11T21:35:02Z

I had to port one logstash_state document to the metricbeat archive

Did that go in another PR? Curious why I'm not seeing a diff to x-pack/test/functional/es_archives/monitoring/logstash_pipelines_mb here

klacabane · 2022-01-12T13:06:37Z

There's a diff for x-pack/test/functional/es_archives/monitoring/logstash_pipelines_mb/data.json.gz where the document was added but the gzipped content won't show up in gh diff, so I added it to a separate non-gzipped file before the final commit for tracking purpose - https://github.com/elastic/kibana/blob/da890dc14e62d6ddf3110db311fdf7e4bcb86520/x-pack/test/functional/es_archives/monitoring/logstash_pipelines_mb/node_stats

klacabane · 2022-01-13T18:54:51Z

@elasticmachine merge upstream

matschaffer · 2022-01-17T04:02:38Z

@elasticmachine merge upstream

matschaffer

I had a few thoughts/questions but overall this looks like an improvement to me so merge when ready!

matschaffer · 2022-01-17T04:05:25Z

x-pack/plugins/monitoring/server/lib/logstash/get_pipeline.ts

@@ -76,6 +76,17 @@ export function _enrichStateWithStatsAggregation(
  statsAggregation: any,
  timeseriesIntervalInSeconds: number
 ) {
+  // we could have data in both legacy and metricbeat collection, we pick the bucket most filled


Wonder if we should consider plotting both? I'd guess this will mean a break in the graph when moving from internal collection to metricbeat.

Since the pipeline stack monitoring is beta though, maybe that's okay.

That could be an option. We could also merge the aggregations and ensure we only consider a single datasource per node, since a pipeline can run on multiple nodes if we receive both legacy and metricbeat monitoring for a pipeline-node tuple there is duplication that we should ignore

That's a really good point. I opened #123191 to capture the improvement for later.

matschaffer · 2022-01-17T04:07:41Z

x-pack/test/functional/services/monitoring/logstash_pipeline_viewer.js

+  const SUBJ_PIPELINE_SECTION_PREFIX = 'pipelineViewerSection_';
+  const PIPELINE_SECTION_ITEM_CLS = 'monPipelineViewer__listItem';
+
+  return new (class LogstashPipelineViewer {


The return new class pattern is really interesting. Not sure why we do it that way, but indeed this fits with the rest of the monitoring test services.

matschaffer · 2022-01-17T04:08:59Z

x-pack/test/functional/services/monitoring/logstash_pipelines.js

+        }
+      }
+
+      throw new Error(`pipeline with id ${id} not found`);


Just nit picking but, seems like this is the pipeline "name" rather than "id". Pretty sure pipelines get a unique id number as well as a string name.

kibana-ci · 2022-01-17T05:21:27Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`monitoring`	445.7KB	445.7KB	+43.0B

History

💚 Build #17351 succeeded d33c637
💚 Build #16735 succeeded 8496d06
💚 Build #16543 succeeded da890dc
💛 Build #16487 was flaky 0b1afc0
💔 Build #16441 failed f6345d9
💔 Build #16165 failed 91ab609

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @klacabane

kibanamachine · 2022-01-17T12:52:38Z

💚 All backports created successfully

Status	Branch	Result
✅	8.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…122443) * add test-subj to pipeline viewer section * pipeline viewer service * clickPipeline * pipeline viewer_mb test * lint * lint * update component snapshot * fix get_pipeline bucket selection * align metricbeat pipeline data with internal monitoring * collect filters * fix pipeline viewer test * clean logstash_pipelines_mb archive Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 6c6a2ed)

…#123149) * add test-subj to pipeline viewer section * pipeline viewer service * clickPipeline * pipeline viewer_mb test * lint * lint * update component snapshot * fix get_pipeline bucket selection * align metricbeat pipeline data with internal monitoring * collect filters * fix pipeline viewer test * clean logstash_pipelines_mb archive Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 6c6a2ed) Co-authored-by: Kevin Lacabane <klacabane@gmail.com>

…122443) * add test-subj to pipeline viewer section * pipeline viewer service * clickPipeline * pipeline viewer_mb test * lint * lint * update component snapshot * fix get_pipeline bucket selection * align metricbeat pipeline data with internal monitoring * collect filters * fix pipeline viewer test * clean logstash_pipelines_mb archive Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

klacabane added 11 commits January 6, 2022 17:09

add test-subj to pipeline viewer section

5ba6383

pipeline viewer service

d281c99

clickPipeline

7b147fa

pipeline viewer_mb test

10bddbd

lint

91ab609

lint

a3ccd1f

update component snapshot

f6345d9

fix get_pipeline bucket selection

c335dab

align metricbeat pipeline data with internal monitoring

d0c335c

collect filters

f35c92b

fix pipeline viewer test

65175e5

Merge branch 'main' into sm-logstash-pipeline-test

0b1afc0

Merge branch 'main' into sm-logstash-pipeline-test

da890dc

clean logstash_pipelines_mb archive

8496d06

klacabane self-assigned this Jan 11, 2022

klacabane added auto-backport Deprecated - use backport:version if exact versions are needed backport-v8.0.0 Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.0.0 v8.1.0 labels Jan 11, 2022

klacabane marked this pull request as ready for review January 11, 2022 17:25

klacabane requested a review from a team as a code owner January 11, 2022 17:25

klacabane changed the title ~~Sm logstash pipeline test~~ [Stack monitoring] logstash - read from correct aggregation Jan 11, 2022

klacabane added the release_note:skip Skip the PR/issue when compiling release notes label Jan 11, 2022

Merge branch 'main' into sm-logstash-pipeline-test

d33c637

Merge branch 'main' into sm-logstash-pipeline-test

4d4a405

matschaffer approved these changes Jan 17, 2022

View reviewed changes

klacabane merged commit 6c6a2ed into elastic:main Jan 17, 2022

kibanamachine mentioned this pull request Jan 17, 2022

[8.0] [Stack monitoring] logstash - read from correct aggregation (#122443) #123149

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Stack monitoring] logstash - read from correct aggregation #122443

[Stack monitoring] logstash - read from correct aggregation #122443

klacabane commented Jan 6, 2022 •

edited

Loading

klacabane commented Jan 10, 2022

klacabane commented Jan 10, 2022

kibanamachine commented Jan 10, 2022

elasticmachine commented Jan 11, 2022

matschaffer commented Jan 11, 2022

klacabane commented Jan 12, 2022 •

edited

Loading

klacabane commented Jan 13, 2022

matschaffer commented Jan 17, 2022

matschaffer left a comment

matschaffer Jan 17, 2022

klacabane Jan 17, 2022

matschaffer Jan 18, 2022

matschaffer Jan 17, 2022

matschaffer Jan 17, 2022

kibana-ci commented Jan 17, 2022

kibanamachine commented Jan 17, 2022

[Stack monitoring] logstash - read from correct aggregation #122443

[Stack monitoring] logstash - read from correct aggregation #122443

Conversation

klacabane commented Jan 6, 2022 • edited Loading

Summary

Testing

klacabane commented Jan 10, 2022

klacabane commented Jan 10, 2022

kibanamachine commented Jan 10, 2022

elasticmachine commented Jan 11, 2022

matschaffer commented Jan 11, 2022

klacabane commented Jan 12, 2022 • edited Loading

klacabane commented Jan 13, 2022

matschaffer commented Jan 17, 2022

matschaffer left a comment

Choose a reason for hiding this comment

matschaffer Jan 17, 2022

Choose a reason for hiding this comment

klacabane Jan 17, 2022

Choose a reason for hiding this comment

matschaffer Jan 18, 2022

Choose a reason for hiding this comment

matschaffer Jan 17, 2022

Choose a reason for hiding this comment

matschaffer Jan 17, 2022

Choose a reason for hiding this comment

kibana-ci commented Jan 17, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Jan 17, 2022

💚 All backports created successfully

Questions ?

klacabane commented Jan 6, 2022 •

edited

Loading

klacabane commented Jan 12, 2022 •

edited

Loading