[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
|
endorama
left a comment
endorama
left a comment
There was a problem hiding this comment.
👍 please add a changelog entry and bump package version to 2.6.0
| 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
* add loadbalancing distribution fields * format fields * update loadbalancing docs * add changelog entry and bump version * add changes to loadbalancing changelog
4 participants
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