Skip to content

Check results in ALS as per downstream/upstream instead of per log#11265

Merged
kezhenxu94 merged 8 commits intoapache:masterfrom
kezhenxu94:meshpluginfallbackperdownupstream
Aug 27, 2023
Merged

Check results in ALS as per downstream/upstream instead of per log#11265
kezhenxu94 merged 8 commits intoapache:masterfrom
kezhenxu94:meshpluginfallbackperdownupstream

Conversation

@kezhenxu94
Copy link
Copy Markdown
Member

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

Currently the mesh analyzer plugins are invoked on every piece of log, and the plugin chain is terminated as soon as any metrics data is extracted from an previous plugin, this is fine if the upstream/downstream from the log both have or have no metadata, then both side can be analyzed in mx plugin or k8s IP mapping plugin, but if one side has metadata and the other side needs k8s IP mapping plugin, the latter will be skipped as the mx plugin already has part of the metrics, this patch will determine the upstream/downstream metrics are extracted or not, and fallback to the next plugin to extract the missing metrics.

@kezhenxu94 kezhenxu94 added backend OAP backend related. enhancement Enhancement on performance or codes labels Aug 25, 2023
@kezhenxu94 kezhenxu94 added this to the 9.6.0 milestone Aug 25, 2023
Comment on lines +91 to +92
return metrics.getHttpMetrics().getMetricsCount() > 0
|| metrics.getTcpMetrics().getMetricsCount() > 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be hasDownstreamMetrics || hasUpstreamMetrics?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is for sidecar analysis, for logs from sidecar. It doesn't care about upstream or downstream. It just cares about whether there is matrics or not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But I can do it to unify the condition to check whether there is metrics

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Meanwhile, do hasDownstreamMetrics/hasUpstreamMetrics have meanings when do sidecar analysis?
I am trying to get variable more accurate.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, you can make sense of the names: for server sidecar there is always only downstream metrics, for client sidecar there is always only upstream metrics.

@wu-sheng
Copy link
Copy Markdown
Member

The CI seems broken. Please fix them.

@kezhenxu94 kezhenxu94 merged commit 16f940b into apache:master Aug 27, 2023
@kezhenxu94 kezhenxu94 deleted the meshpluginfallbackperdownupstream branch August 27, 2023 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related. enhancement Enhancement on performance or codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants