-
Notifications
You must be signed in to change notification settings - Fork 529
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
model: add system.network.{cpus,memory,network} #4912
Conversation
@vigneshshanmugam here's a start on the new fields for RUM device info. I started adding a new top-level "browser" field to metadata, but it occurred to me that this is probably also going to be relevant to mobile devices, and in that case there may not be any browsers involved. @bryce-b would we want to capture these properties for mobile? Still not sure where to put the network info in the Elasticsearch schema. I'd appreciate any thoughts anyone may have. |
Codecov Report
@@ Coverage Diff @@
## master #4912 +/- ##
==========================================
- Coverage 76.57% 76.22% -0.36%
==========================================
Files 166 166
Lines 10135 10160 +25
==========================================
- Hits 7761 7744 -17
- Misses 2374 2416 +42
|
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Test errors
Expand to view the tests failures> Show only the first 10 test failures
|
This looks really good.
Yeah very good point. Is there any harm keeping them in the top level as |
docs/spec/v2/metadata.json
Outdated
] | ||
}, | ||
"effectivetype": { | ||
"description": "EffectiveType describes the effective network type as one of a discrete set of network types, e.g. 'slow-2g', '2g', '3g', or '4g'. See https://developer.mozilla.org/en-US/docs/Web/API/NetworkInformation for more details.", |
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.
should we move this URL under network
so all fields can be referenced from there.
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, done.
Those are a little bit too generic for my tastes. In theory we might later want to enable backend services to record network information on behalf of their clients. If we did that it would make more sense to nest under I just had a quick look and there has been a discussion about adding device fields to ECS (elastic/ecs#448), but the conclusion was to use I'm inclined to record the data as Thus in the ES docs we would end up fields like
WDYT? |
Sounds good to me. We need to come up with a shorthand fields for the v3 spec and should be good to go. |
@vigneshshanmugam yes, it's just a matter of prioritisation. For OTel we decided to stick with lower_snake_case names and so I think we should do the same here. We ended up with |
Yes totally we can use the same naming scheme from OTEL. But for RUM, we are going to be using shortened fields for the V3 spec so it should not be a huge concern from the agent perspective.
I don't think we have any plans of adding the fields for 7.14 @drewpost Correct me if am wrong, But probably if we can include them as part of 7.15, that should be a good to include them as part of Exploratory view dashboard. |
That's fine, I was just talking about the field name in Elasticsearch. We can still come up with a shortened name for v3. |
Sounds good. ++ to keeping it as |
This pull request does not have a backport label. Could you fix it @axw? 🙏
NOTE: |
This pull request is now in conflicts. Could you fix it @axw? 🙏
|
This is very stale. It's unclear what the priority is, so let's come back to this when it is clearer. A design doc would be great. |
Motivation/summary
Extend the system metadata with properties for describing the number of CPU cores, amount of system memory, and network information for end-user devices.
Checklist
How to test these changes
TODO
Related issues
#3657