[GCP] Add loadbalancing_metrics distribution fields #4004
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
🌐 Coverage report
|
| @@ -59,3 +59,43 @@ | |||
| - name: tcp_ssl_proxy.open_connections.value | |||
| type: long | |||
| description: Current number of outstanding connections through the TCP/SSL proxy. | |||
| - name: https.backend_latencies.value | |||
| type: object | |||
| object_type: histogram | |||
There was a problem hiding this comment.
Why does this need to use object_type? Could it simply use type: histogram?
In what stack versions is object_type: histogram supported? That seems like something new.
There was a problem hiding this comment.
I used the prometheus integration as a reference. Should I just use type: histogram instead?
There was a problem hiding this comment.
From my understanding object_type should only be used for type: array fields, see https://github.com/elastic/package-spec/blob/34c28eedce0d1ae7c140c998dd1e1bfdccdfc578/versions/1/integration/data_stream/fields/fields.spec.yml#L236-L239
So I think object_type: histogram is supported, but should only be used if the field is of type: array.
@andrewkroh if you confirm this I think this should be changed.
There was a problem hiding this comment.
Is there an example document somewhere that I can review to see what the fields look like?
There was a problem hiding this comment.
By looking at this documentation I think you are right type: histogram is a valid type.
There was a problem hiding this comment.
After researching this topic for a bit, my conclusion is that: object_type: histogram is supported but is not clear to me if type: histogram would produce the same result.
@gpop63 could you try using type: histogram to see if the behaviour is the same?
There was a problem hiding this comment.
I tested the way @endorama suggested and using type: histogram in the integration field type does not work (reason":"error parsing field [gcp.loadbalancing.l3.external.rtt_latencies.value], with unknown parameter [histogram]") maybe because in metricbeat they were added as type: object. I think we should either leave it like this or try changing metric types in metricbeat to type: histogram.
There was a problem hiding this comment.
I also raised 2 issues with the team behind package-spec for missing documentation and they fixed it super fast with this PR, confirming support for object_type.
@andrewkroh Let us know if you still have doubts around this, otherwise I think we should proceed by using object_type.
…istribution_metrics
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
dns |
1808.32 | 1524.39 | -283.93 (-15.7%) | 💔 |
loadbalancing_logs |
3937.01 | 3333.33 | -603.68 (-15.33%) | 💔 |
To see the full report comment with /test benchmark fullreport
What does this PR do?
Adds
loadbalancing_metricsmissing distribution fields.Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots