Add system network fields #5436

jalvz · 2021-06-10T13:32:10Z

Motivation/summary

See motivation on the linked ticket bellow.

Some notes:

Uses network instead of net because ECS prefers it.
Does not have a host subkey, like in the OTel spec proposal because ECS already has a top level host key. We could nest network inside host but I don't see any value in doing that.
Does not add RUM v3 fields.

Checklist

Update CHANGELOG.asciidoc
Documentation has been updated

How to test these changes

Related issues

Closes #5203

apmmachine · 2021-06-10T13:41:22Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #5436 updated
Start Time: 2021-06-16T11:52:34.440+0000
Duration: 40 min 42 sec
Commit: 8a24fa5

Test stats 🧪

Test	Results
Failed	0
Passed	6094
Skipped	120
Total	6214

Trends 🧪

axw

Fields and processor/otel changes LGTM, but I don't think we need the modeldecoder, processor/stream, or testdata changes.

axw · 2021-06-14T01:57:22Z

model/modeldecoder/v2/decoder.go

@@ -526,6 +526,21 @@ func mapToMetadataModel(from *metadata, out *model.Metadata) {
 	if from.System.Platform.IsSet() {
 		out.System.Platform = from.System.Platform.Val
 	}
+	if from.System.Network.Carrier.Name.IsSet() {


The new fields are only for the iOS agent, which uses OTLP. Let's leave modeldecoder out of it please.

model/system_test.go

processor/otel/metadata.go

Co-authored-by: Andrew Wilkins <axwalk@gmail.com>

simitt · 2021-06-15T13:59:48Z

apmpackage/apm/data_stream/app_metrics/fields/fields.yml

+- name: network.connection_type
+  type: keyword
+  description: |
+    Cellular network technology, eg. 4G


I am not certain about the naming. ECS has a definition for network and one for host with some host.network fields. None of them contain what we need. This PR introduces network.carrier.* fields, that are not (yet) in ECS, but also not aligned with the otel proposal net.host.carrier.*.

@jalvz did you have a conversation with the ECS folks where they would see these fields fit in? I am a bit worried to just remove the host part, that is in the otel proposal.

We have already a top level host field, I didn't think it would make much sense to duplicate the field inside network to refer to the same host?
network is already defined as top level field in ECS (https://www.elastic.co/guide/en/ecs/current/ecs-network.html),

I sort of followed #491, btw.

ECS says (about the network fieldset):

The network is defined as the communication path over which a host or network event happens.

The network.* fields should be populated with details about the network activity associated with an event.

Seems to me it makes sense to go under network

So let me ask:

host and network.host would refer to the same host, or different ones?

If different: which is what? If same: why duplicate a field we already have?

What exactly says in the ECS network definition that dictates there should be a host field inside network? (sorry I can't see that in the quote)

If we go with network.host, why don't we apply the same reasoning elsewhere? Eg., a process normally runs on a host, yet there is no process.host; rather 2 top fields (conceptually related).

If we go with network.host, how do you justify that in the context of other fields? We could have eg. network.host.connection_type="wifi" and network.name="Guest wifi" - why connection_type and name would not be at the same level?

What does it mean, conceptually, network.host.carrier?: a host has a carrier? a carrier runs on a host?...

After some offline discussion, network.carrier.* is where the new fields fit best related to ECS, and net.host.carrier.* was the prefered nesting in the open-telemetry proposal; so 👍 on the changes.

mergify · 2021-06-16T05:26:45Z

This pull request is now in conflicts. Could you fix it @jalvz? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b mobile-fields upstream/mobile-fields
git merge upstream/master
git push upstream mobile-fields

axw

LGTM, but please wait until we have consensus on #5436 (review)

(cherry picked from commit 01a12f2) # Conflicts: # changelogs/head.asciidoc # include/fields.go

stuartnelson3 · 2021-06-29T09:45:06Z

Added test-plan to this one, but feel free to remove it

simitt · 2021-07-13T20:08:09Z

Tested with BC 2 - network fields are not indexed into ES although labels are present.

stuartnelson3 · 2021-07-19T18:58:18Z

Bumped to 7.15 in #5719

add system network fields

b58556b

jalvz force-pushed the mobile-fields branch from 2be7010 to b58556b Compare June 10, 2021 20:08

axw requested changes Jun 14, 2021

View reviewed changes

jalvz and others added 5 commits June 14, 2021 14:42

code review

010925b

Update model/system_test.go

01c456a

Co-authored-by: Andrew Wilkins <axwalk@gmail.com>

Update processor/otel/metadata.go

c2add24

Co-authored-by: Andrew Wilkins <axwalk@gmail.com>

liiiiint

f9433e7

add changelog

d69e467

jalvz added the v7.14.0 label Jun 15, 2021

jalvz added 2 commits June 15, 2021 15:01

Merge branch 'master' into mobile-fields

d36f30f

Merge branch 'master' into mobile-fields

6145741

simitt reviewed Jun 15, 2021

View reviewed changes

Merge branch 'master' into mobile-fields

d0d5b06

jalvz requested a review from axw June 16, 2021 07:30

axw approved these changes Jun 16, 2021

View reviewed changes

simitt approved these changes Jun 16, 2021

View reviewed changes

Merge branch 'master' into mobile-fields

8a24fa5

jalvz merged commit 01a12f2 into elastic:master Jun 16, 2021

mergify bot pushed a commit that referenced this pull request Jun 16, 2021

Add system network fields (#5436)

b2fefc4

(cherry picked from commit 01a12f2) # Conflicts: # changelogs/head.asciidoc # include/fields.go

mergify bot mentioned this pull request Jun 16, 2021

[7.x] Add system network fields (backport #5436) #5467

Merged

vigneshshanmugam mentioned this pull request Jun 16, 2021

model: add system.network.{cpus,memory,network} #4912

Closed

2 tasks

jalvz pushed a commit that referenced this pull request Jun 17, 2021

[7.x] Add system network fields (backport #5436) (#5467)

0653c21

stuartnelson3 added the test-plan label Jun 29, 2021

axw mentioned this pull request Jul 12, 2021

Update network connection fields #5671

Merged

1 task

simitt self-assigned this Jul 12, 2021

simitt added the test-plan-regression label Jul 13, 2021

axw mentioned this pull request Jul 15, 2021

processor/otel: handle net.host.* as span attributes #5719

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add system network fields #5436

Add system network fields #5436

jalvz commented Jun 10, 2021 •

edited

Loading

apmmachine commented Jun 10, 2021 •

edited

Loading

Build stats

Test stats 🧪

Trends 🧪

axw left a comment

axw Jun 14, 2021

simitt Jun 15, 2021

jalvz Jun 15, 2021

axw Jun 16, 2021

jalvz Jun 16, 2021 •

edited

Loading

simitt Jun 16, 2021

mergify bot commented Jun 16, 2021

axw left a comment

stuartnelson3 commented Jun 29, 2021

simitt commented Jul 13, 2021

stuartnelson3 commented Jul 19, 2021

Add system network fields #5436

Add system network fields #5436

Conversation

jalvz commented Jun 10, 2021 • edited Loading

Motivation/summary

Checklist

How to test these changes

Related issues

apmmachine commented Jun 10, 2021 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

axw left a comment

Choose a reason for hiding this comment

axw Jun 14, 2021

Choose a reason for hiding this comment

simitt Jun 15, 2021

Choose a reason for hiding this comment

jalvz Jun 15, 2021

Choose a reason for hiding this comment

axw Jun 16, 2021

Choose a reason for hiding this comment

jalvz Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

simitt Jun 16, 2021

Choose a reason for hiding this comment

mergify bot commented Jun 16, 2021

axw left a comment

Choose a reason for hiding this comment

stuartnelson3 commented Jun 29, 2021

simitt commented Jul 13, 2021

stuartnelson3 commented Jul 19, 2021

jalvz commented Jun 10, 2021 •

edited

Loading

apmmachine commented Jun 10, 2021 •

edited

Loading

jalvz Jun 16, 2021 •

edited

Loading