model: ECS for 6.x #1609

graphaelli · 2018-11-30T03:23:10Z

Make new 6.x indices look like they will in 7.x by aliasing fields where possible and copying data where necessary.

Tasks:

All non-indexed fields should be documented now
fields pending decisions on aliasing: context.http.status_code context.response.status_code and context.response.finished
decision on user.user_agent pipeline (seeking confirmation this is no-op since it's not indexed and user configurable)
Restore host processor metadata fields
merge in the latest beats framework update ([6.x] Update beats framework to c63c98b #1647) now that ECS fields have landed there, no major issues expected. The host metadata fields will be restored at that time.

Reviewers: please pay particular attention to ensure the meanings of the fields in ECS is respected - that is that not only do the data types match, but the values will be usable across applications. Also please look for an opportunities to fill in in event.* that might have been missed.

Elasticsearch aliases are leveraged for ECS compatibility wherever possible. Where that's not possible, values are written to both the original location and the one ECS dictates. Here: * copy context.tags to labels - object fields can't be aliased * cast & copy context.request.url.port to url.port - keyword -> int * copy context.request.url.protocol to url.scheme - trim the final :

_meta/ecs-migration.yml

@webmat

url.full -> context.request.url.full url.original -> context.request.url.raw per feedback from @webmat

webmat

Looking good so far. Please ping me on any subsequent mapping work.

docs/fields.asciidoc

graphaelli · 2018-12-03T15:55:50Z

backport of elastic/beats#9269 will fix the bad docs generated for aliased fields.

webmat · 2018-12-03T15:57:57Z

@graphaelli Just clarifying: host.hostname is meant to contain exactly what the command hostname would return on that server. host.name can also default to this value, but is the field a user would override, if they decide to do so.

graphaelli · 2018-12-03T16:10:11Z

@webmat Can you check my understand here - in the APM use case host.hostname would be the hostname of the application server being instrumented and host.name would default to that but possibly overridden by a user. Also, we'll need to notify users that host.name in <6.6 was the apm-server's hostname but now will be the application server's. The apm-server hostname has been moved under agent.

Note that the application server is the browser in the RUM case.

model/metricset/event.go

webmat · 2018-12-03T18:31:03Z

@graphaelli We are currently reviewing whether agent.hostname will make it into ECS. I can't answer that yet.

cc @MikePaquette

simitt

What is the idea for all values under error, transaction, span, metric?
The category event troubles me especially, as it is described to hold the context of an event, eg event.start, event.duration, etc. We do have this information inside transaction, span, etc.
In case we moved the data from our events into the event namespace, we would need to find a solution for how to refer to parent, trace, transaction (from error and span event) etc. I was thinking we could make use of related here, but not sure I completely understood the related field.

_meta/fields.common.yml

_meta/ecs-migration.yml

_meta/fields.common.yml

ruflin

Hm, forgot some time ago to hit submit ...

_meta/ecs-migration.yml

_meta/fields.common.yml

vendor/github.com/elastic/beats/libbeat/publisher/pipeline/module.go

@simitt

per feedback from @simitt

tests/system/test_ecs_mappings.py

This reverts commit 99c19c7.

simitt

I haven't looked at tests/system/test_ecs_mappings.py closely yet, but reviewed everything else.

_meta/ecs-migration.yml

simitt · 2018-12-14T08:58:18Z

_meta/ecs-migration.yml

+  copy_to: true
+
+- from: context.process.argv
+  to: process.args


I know I brought it up, but as this is actually not indexed, I think we should remove it again from here, or set alias=false.

It should be indexed in an ECS index.

Perhaps if APM doesn't make this useable this requirement can be relaxed? @ruflin WDYT?

Good point, removed altogether since it's a useless (though allowed, somewhat surprisingly) alias.

_meta/ecs-migration.yml

simitt · 2018-12-14T09:13:21Z

_meta/fields.common.yml

+            # - name: socket.encrypted
+            #   type: boolean
+            # - name: socket.remote_address
+            #   type: keyword


Is it possible to also define the nested fields here?

I want to so they'll show up in documentation but not as-is. Did you have other concerns besides docs?

No concerns, I was hoping we could add them.

simitt · 2018-12-14T09:44:59Z

_meta/fields.common.yml

+              searchable: false
+              doc_values: false
+
+            - name: status_code


This is an indexed field, same datatype as context.response.status_code that is aliased to http.response.status_code now. I think this should also be mapped to the same field, by copying it over instead of aliasing.

I convinced myself that this field is metadata on a span and might actually be confusing if it's in http.response.status_code, as it's the response code from calling a remote service vs the response code we return from the traced service.

If you would still like to proceed, are you saying that in:
6.6 - put this value in context.response.status_code and context.http.status_code
7.0 - write it to http.response.status_code

I can see the ambiguity here. This is to track the response of an external call performed by the monitored app, correct?

I think I'd be inclined to still put it in the ECS field. After all, these events will mostly be looked at in the context of this one event stream. Might as well benefit from the streamlined field names.

But if you see a problem with this, we can hold off on the migration of this field.

This is to track the response of an external call performed by the monitored app, correct?
correct

I don't have philosophical objections to this plan - my concern is mostly a practical issue of this being our only 1:many field. I agree that in 7.0.0 we can write this to http.response.status_code, after all. an APM event stream in the distributed tracing case is composed of multiple transactions that will each bring a possibly different http.response.status_code value. It's also not a big effort to write this to both places for spans, I just want to be sure we want that in 6.6 because I'm not sure what the benefit of that is, since we can't make that work for pre-6.6 indices.

ok, I am good with leaving as is for 6.x and reconsider for 7.0

model/ecs.go

Conflicts: docs/data/elasticsearch/generated/errors.json docs/data/elasticsearch/generated/transactions.json processor/stream/approved-es-documents/testV2IntakeIntegrationErrors.approved.json processor/stream/approved-es-documents/testV2IntakeIntegrationTransactions.approved.json processor/stream/package_tests/error_attrs_test.go processor/stream/package_tests/transaction_attrs_test.go tests/fields.go

@simitt

per feedback from @simitt, turns out context.process.argv is not indexed so alias does not make sense

webmat

Noticed a few more things

webmat · 2018-12-14T16:54:36Z

_meta/ecs-migration.yml

+  copy_to: true
+
+- from: context.process.argv
+  to: process.args


It should be indexed in an ECS index.

Perhaps if APM doesn't make this useable this requirement can be relaxed? @ruflin WDYT?

_meta/ecs-migration.yml

webmat · 2018-12-14T17:07:02Z

_meta/ecs-migration.yml

+  to: client.ip
+
+- from: context.user.user-agent
+  to: user_agent.original.text


user_agent.original.text would be a multi-field, which is defined in the context of the "real" field, which is user_agent.original in this case.

I guess we're hitting a corner case here. The field (in terms of setting an alias) is moving to user_agent.original. I'm not sure it's possible to create an alias to a multi-field. We need to look into this.

But the usage side, if APM intends to use this as "what field does the query target", then yeah it's now user_agent.original.text.

cc @ruflin

you can alias to a multi-field but not back from a multi-field (marked as a non-reversible alias)

_meta/ecs-migration.yml

model/ecs.go

graphaelli · 2018-12-14T18:02:12Z

It should be indexed in an ECS index.
Perhaps if APM doesn't make this useable this requirement can be relaxed? @ruflin WDYT?

We've decided it's not worth indexing this field - it's often huge (think java, where it's not even captured by default) and uninteresting for queries - other fields are better suited for search/aggs, like process.name, and then args are handy metadata per document.

graphaelli · 2018-12-15T16:24:07Z

After further consideration of elastic/beats#9412 (comment), removing the context.tags -> labels copy. There is still time to restore it, even behind a flag, if desired.

simitt · 2018-12-17T09:00:36Z

@graphaelli I don't quite understand what the problem with copying context.tags to labels is based on customer defined keys. I though we would copy all fields nested under `context.tags.

Apart from that, this PR LGTM.

graphaelli · 2018-12-17T14:04:46Z

The main concern is that it may surprisingly take a significant amount of space. I'd like to merge this and follow up after we discuss a bit further on a separate issue.

roncohen · 2018-12-17T14:06:58Z

Exciting! Great work!

graphaelli added 4 commits November 30, 2018 13:54

add ECS aliases

a6af765

exclude aliases

701050d

take ownership of host.name

99c19c7

graphaelli force-pushed the ecs-6.x branch from abdd459 to 99c19c7 Compare November 30, 2018 18:57

graphaelli requested a review from ruflin November 30, 2018 19:24

webmat reviewed Nov 30, 2018

View reviewed changes

_meta/ecs-migration.yml Outdated Show resolved Hide resolved

update request url fields

75a4085

url.full -> context.request.url.full url.original -> context.request.url.raw per feedback from @webmat

graphaelli force-pushed the ecs-6.x branch from 35ce28f to 75a4085 Compare November 30, 2018 21:21

graphaelli mentioned this pull request Dec 3, 2018

[WIP] Map ECS fields #1540

Closed

map meta.cloud.*

4c40f12

webmat reviewed Dec 3, 2018

View reviewed changes

docs/fields.asciidoc Outdated Show resolved Hide resolved

webmat reviewed Dec 3, 2018

View reviewed changes

model/metricset/event.go Show resolved Hide resolved

alvarolobato assigned graphaelli Dec 4, 2018

alvarolobato added [zube]: Inbox and removed [zube]: Inbox labels Dec 4, 2018

simitt reviewed Dec 5, 2018

View reviewed changes

ruflin reviewed Dec 5, 2018

View reviewed changes

graphaelli added 4 commits December 5, 2018 15:15

Merge remote-tracking branch 'elastic/6.x' into ecs-6.x

bb6dd7b

account for new fields

e122d1d

add fields missing from migration log + test

3a7330a

per feedback from @simitt

copy context.process.argv to process.args

4c4f153

simitt reviewed Dec 6, 2018

View reviewed changes

tests/system/test_ecs_mappings.py Outdated Show resolved Hide resolved

graphaelli added 2 commits December 6, 2018 15:23

alias host.hostname to context.system.hostname, instead of host.name

4702718

This reverts commit 99c19c7.

fix process.args exception

1b24cdd

graphaelli added 4 commits December 13, 2018 16:46

alias http.response.finished -> context.response.finished

629356d

mapping complete?

c45b98d

pep8

2a8fecf

alias process.args -> context.process.argv instead of copying

1004b8f

graphaelli changed the title ~~WIP model: ECS for 6.x~~ model: ECS for 6.x Dec 14, 2018

graphaelli added [zube]: In Review and removed [zube]: In Progress labels Dec 14, 2018

simitt reviewed Dec 14, 2018

View reviewed changes

graphaelli added 2 commits December 14, 2018 11:49

remove process.args

cd12363

per feedback from @simitt, turns out context.process.argv is not indexed so alias does not make sense

webmat reviewed Dec 14, 2018

View reviewed changes

unit test ECS transformations on transactions

304bcc7

remove context.tags -> labels copy

fb54f5e

graphaelli merged commit 7cae5b1 into elastic:6.x Dec 17, 2018

graphaelli deleted the ecs-6.x branch December 17, 2018 14:05

zube bot added [zube]: Done and removed [zube]: In Review labels Dec 17, 2018

graphaelli added a commit to graphaelli/kibana that referenced this pull request Dec 18, 2018

[APM] update index pattern for elastic/apm-server#1609

fed57a1

graphaelli mentioned this pull request Dec 18, 2018

[APM] update index pattern elastic/kibana#27434

Merged

graphaelli added a commit to elastic/kibana that referenced this pull request Dec 19, 2018

[APM] update index pattern for elastic/apm-server#1609 (#27434)

f3fcd53

simitt mentioned this pull request Jan 11, 2019

fill in ECS-compat top-level fields from context #1766

Merged

This was referenced Jan 16, 2019

Remove aliases in 6.6 after decision to go with a reindex strategy #1794

Closed

[6.6] Remove ECS aliases #1795

Merged

simitt mentioned this pull request Jul 30, 2019

ECS: Set host.name along with host.hostname in exported data #2502

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

model: ECS for 6.x #1609

model: ECS for 6.x #1609

graphaelli commented Nov 30, 2018 •

edited

Loading

webmat left a comment

graphaelli commented Dec 3, 2018

webmat commented Dec 3, 2018

graphaelli commented Dec 3, 2018

webmat commented Dec 3, 2018

simitt left a comment

ruflin left a comment

simitt left a comment

simitt Dec 14, 2018

webmat Dec 14, 2018

graphaelli Dec 14, 2018 •

edited

Loading

simitt Dec 14, 2018

graphaelli Dec 14, 2018

simitt Dec 17, 2018

simitt Dec 14, 2018

graphaelli Dec 14, 2018

webmat Dec 14, 2018

graphaelli Dec 14, 2018

simitt Dec 17, 2018

webmat left a comment

webmat Dec 14, 2018

webmat Dec 14, 2018

graphaelli Dec 14, 2018

graphaelli commented Dec 14, 2018

graphaelli commented Dec 15, 2018

simitt commented Dec 17, 2018

graphaelli commented Dec 17, 2018

roncohen commented Dec 17, 2018

model: ECS for 6.x #1609

model: ECS for 6.x #1609

Conversation

graphaelli commented Nov 30, 2018 • edited Loading

webmat left a comment

Choose a reason for hiding this comment

graphaelli commented Dec 3, 2018

webmat commented Dec 3, 2018

graphaelli commented Dec 3, 2018

webmat commented Dec 3, 2018

simitt left a comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

simitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graphaelli Dec 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webmat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graphaelli commented Dec 14, 2018

graphaelli commented Dec 15, 2018

simitt commented Dec 17, 2018

graphaelli commented Dec 17, 2018

roncohen commented Dec 17, 2018

graphaelli commented Nov 30, 2018 •

edited

Loading

graphaelli Dec 14, 2018 •

edited

Loading