-
Notifications
You must be signed in to change notification settings - Fork 99
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
Docs: Create network metrics docs #675
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #675 +/- ##
==========================================
- Coverage 77.40% 76.95% -0.45%
==========================================
Files 96 96
Lines 8016 8016
==========================================
- Hits 6205 6169 -36
- Misses 1470 1507 +37
+ Partials 341 340 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I wouldn't worry about the prose to be honest, I found it too restrictive for my language and style, but if you want to fight and get it through do it :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've committed an edit for _index.md
, and quickstart.md
.
I have a few questions, and still would like to do another light pass to quickstart.md
and full pass on config.md
.
docs/sources/network/_index.md
Outdated
|
||
## Metric attributes | ||
|
||
Network metrics provides a single **OpenTelemetry** metric `beyla.network.flow.bytes`, a counter of the total bytes sent of network flows observed by the eBPF probe since its launch, with the following attributes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I edited the sentence but kept the wording, "total bytes sent of network flows observed by the eBPF probe since its launch", but it feels a little unclear. Could someone explain this phrase in another way so I can make sure we capture the description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can just say "a counter of the number of bytes observed between two network endpoints"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that 👍
docs/sources/network/_index.md
Outdated
|
||
Network metrics provides a single **OpenTelemetry** metric `beyla.network.flow.bytes`, a counter of the total bytes sent of network flows observed by the eBPF probe since its launch, with the following attributes: | ||
|
||
- `beyla.ip`: the local IP address of the Beyla instance that emitted the metric |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the attribute pairs mutually exclusive or can they both occur? Could we separate them to their own lines, even better can we use a table?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They can Both occur. I will use a table, thanks for the suggestion!
name: grafana-secret | ||
``` | ||
|
||
## Select metrics attributes to reduce cardinality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we going to add content for these two sections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a WIP PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! With a few comments :)
Lets ship this today!
### Allowed attributes | ||
|
||
If the metric with all the attributes is reported it might lead to a cardinality explosion, especially when including external traffic in the `src.address`/`dst.address` attributes. | ||
|
||
You can specify which attributes are allowed in the Beyla configuration. Allowed attributes and aggregates the metrics by them. For example: | ||
|
||
```yaml | ||
network: | ||
enable: true | ||
allowed_attributes: | ||
- k8s.src.owner.name | ||
- k8s.src.namespace | ||
- k8s.dst.owner.name | ||
- k8s.dst.namespace | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the default set while others can enabled explicitly by the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on it on another PR and later I'll amend this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished editing.
No description provided.