Skip to content
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

Beta label support #1051

Merged
merged 20 commits into from
Nov 18, 2020
Merged

Beta label support #1051

merged 20 commits into from
Nov 18, 2020

Conversation

ebeahan
Copy link
Member

@ebeahan ebeahan commented Oct 26, 2020

Summary

Introduces a new key, beta, which when set to True designates a field as being in beta support in the ECS field reference documentation.

Motivation

Fields introduced in RFCs which have advanced to stage three are to be committed to the schema as beta. The "beta" label should appear in the ECS docs next to any fields which are in beta.

The ECS field definitions need an option to specify if a field is currently considered beta. Once an RFC advances to stage four, the beta label will be removed by removing the beta option from the field's definition.

Implementation

A field can be marked as beta by specifying the beta option:

    - name: executable
      level: extended
      type: keyword
      beta: True
      description: >
        Absolute path to the process executable.
      example: /usr/bin/ssh
      multi_fields:
      - type: text
        name: text

Field Reference Docs Example

Once the field reference documentation is regenerated using make, the field now appears in the docs with the beta label if we pass text to the beta:[] argument:

Screen Shot 2020-10-26 at 2 48 37 PM

The label is using the default tooltip text, but we can optionally customize the contents.

Screen Shot 2020-10-26 at 2 48 44 PM

@ebeahan ebeahan self-assigned this Oct 26, 2020
@ebeahan ebeahan added the 1.8.0 label Oct 26, 2020
@ebeahan
Copy link
Member Author

ebeahan commented Oct 27, 2020

Also shared in elastic/docs#1754 (comment)

There's currently a CSS-related issue in how the generated docs will display the beta:[] tooltip inside a table. Depending on where the [beta] label is placed in a fieldset's table, part of the tooltip may get cut off by the bounds of the table.

Screen Shot 2020-10-27 at 1 36 30 PM

@ebeahan ebeahan marked this pull request as ready for review October 27, 2020 18:55
@ebeahan ebeahan force-pushed the add-beta-marker branch 2 times, most recently from e1588bf to 717b277 Compare October 29, 2020 20:50
@ebeahan ebeahan requested a review from webmat October 29, 2020 20:59
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does a good job for any beta fields.

However I'm wondering if automating this beta tagging will be a complex rabbit-hole. Here's various situations I see, with regards to beta concepts in ECS:

  • We have a whole section in beta, in the categorization fields. Although it's already marked beta in another way, so we can ignore this.
  • RFC 0007 [RFC] Multiple users in an event, stage 3 PR #1017 will introduce beta field reuses
  • RFC 0001 [RFC] Wildcard - stage 3 proposal #1015 will migrate existing non-beta fields to a different type. But some of these fields have been around since ECS 0.1.0, so they're not beta fields, only their usage of the new type is :-)
  • This will work for most other RFCs in flight, although I could see a desire to flag a whole field set as beta as well.

This opens up a few questions.

Field reuses: I think we could potentially add support inside the small reuse object to identify that this is a beta reuse:

  reusable:
    expected:
      - at: user
        as: target
        beta: true

Field type: this one I'm not sure how we should mark a transition like: used to be keyword and is now wildcard.

Field set: Not sure if we'd want to have the beta label appear on every single field in a new beta field set. A plain text notice might be better at the top of the field set.

With all of these edge cases -- and I might be forgetting others -- I wonder if we shouldn't try to rethink this with a more basic approach.

Perhaps something like a beta notice at the field set level, that takes text (just like the description field). I think this would let us most edge cases quite simply.

Field reuses:

- name: user
  title: User
  beta: >
    Note that the new field reuses `user.effective`, `user.target` and `user.changes` are beta.

Type changes:

- name: user
  title: User
  beta: >
    Note that the new field reuses `user.effective`, `user.target` and `user.changes` are beta.
    Note that usage of type `wildcard` for fields `user.name`, ... is considered beta. These fields used to be using type `keyword`.

Note that these "type change" beta notices would have to be applied to each field set modified.

Note also that by ECS 1.8, the user field set will likely need both of these notices. This approach lets us deal with this in a simple manner with text. Although I'm not sure if having this single growing notice actually becomes a problem with my approach.

Field set:

Any time a whole field set is introduced, we can reuse the same boilerplate text.

- name: Foo
  title: Foo
  beta: >
    Note that this field set was recently introduced, and is considered beta.

I'm still on the fence for one off fields being marked beta, however. With my approach we could absolutely have a beta paragraph that calls out beta fields at the top. However when specific fields are beta, I think it's also great to have a mention in context. So perhaps we should keep the current approach, and pair it with a field set notice that calls out to them?

scripts/templates/field_details.j2 Outdated Show resolved Hide resolved
@ebeahan
Copy link
Member Author

ebeahan commented Oct 30, 2020

I can see the simplicity of containing everything to the field set level. It would also let us workaround the table formatting issue I mentioned above 😄 .

We had a discussion recently around making each field directly linkable. I can also see value in having the individuals fields tagged with a beta marker for that case. Avoids any chance for a user to visit the direct link and fails to realize the field is beta since they didn't happen to scroll up to view the field set level header. However, it does feel unnecessary to include beta on every field in an entirely beta field set. 🤔

If we agree to leave the beta marker for individual fields, we do have the ability to control the text in the tooltip. Perhaps we can use this feature to better explain when beta is referring to a type migration vs. a net new field?

(Again, though, adds some additional complexity of handling the custom text for a particular subset of fields).

Screen Shot 2020-10-30 at 1 38 25 PM

I do agree we will eventually need support for a field set level notice regardless, and if we do collectively decide to continue marking specific fields, I think also having the field set level beta notice too is a good add.

Any objections to implementing the field set level similar to this mock-up? The text would be populated based on the field set level beta description.

- name: agent
  title: Agent
  beta: >
    Note that this field set was recently introduced, and is considered beta.

Screen Shot 2020-10-30 at 1 42 03 PM

For the field reuse cases, I think having beta called out somewhere underneath the Field Reuse section is helpful. Each field reuse can already be linked to directly in the current docs. I also think it's clearer to call out when a field reuse is considered beta underneath the Field Reuse section vs. the very top of the field set.

I put together a couple of mocks again to give an idea of what the end results could be:

  • Beta fields reuse section header. Text specifies which field reuses are considered beta.

Screen Shot 2020-10-30 at 2 09 55 PM

  • Marking each field nesting beta individually:

Screen Shot 2020-10-30 at 1 49 27 PM

@ebeahan ebeahan added the ready Issues we'd like to address in the future. label Nov 3, 2020
@ebeahan
Copy link
Member Author

ebeahan commented Nov 5, 2020

I've taken an approach that feels a bit "All the Things!!", but I like that it offers maximum flexibility in methods available for beta labeling.

Placement

The [beta] marker now appears on its own line in the description field vs. the field name. I felt this was a cleaner look. It also eliminates many instances of the CSS issue mentioned earlier #1051 (comment).

Attributes

Fieldsets

Added a beta fieldset-level attribute which adds a beta warning for the entire fieldset. The text provided in the beta attribute value becomes the contents of the marker:

- name: user
  ...
  beta: >
    This fieldset is currently beta

Screen Shot 2020-11-05 at 11 10 36 AM

Fields

The field level attribute remains. The text provided in the attribute will become the contents of the beta tooltip.

    - name: name
      ...
      beta: >
        This field is currently beta.

Screen Shot 2020-11-05 at 11 13 07 AM

Field Reuse

For field reuses, I've added support for a beta key under the expected object. This has the drawback of not supporting the flat-style notation, but this can be worked around by using the longer object notation.

The text from beta again is used for the tooltip.

  reusable:
    top_level: true
    expected:
      ...
      - at: user
        as: target
        beta: >
          This is a beta reuse field

Screen Shot 2020-11-05 at 11 18 51 AM

@ebeahan ebeahan requested a review from webmat November 5, 2020 17:27
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love where this is going!

I like the option in field reuses. Don't worry about the flat list, it's now just a shorthand for the most straightforward reuses. The "real" reuse notation now is the object one, so whenever we add features there, we should only worry about the object notation.

Also, between the two renderings of the beta notice for field reuse, I much prefer the one in context ("Marking each field nesting beta individually"). The other one will be too easy to miss, IMO.

Do all 3 new displays of the beta text deal well with multiple paragraphs? If not, perhaps we should watch out for that in the script, and break (in --strict mode).

CHANGELOG.next.md Outdated Show resolved Hide resolved
schemas/README.md Outdated Show resolved Hide resolved
schemas/README.md Outdated Show resolved Hide resolved
scripts/templates/field_details.j2 Show resolved Hide resolved
scripts/tests/unit/test_schema_finalizer.py Outdated Show resolved Hide resolved
schemas/README.md Outdated Show resolved Hide resolved
webmat
webmat previously approved these changes Nov 18, 2020
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ebeahan
Copy link
Member Author

ebeahan commented Nov 18, 2020

Thanks @webmat!

To align with the other changes, I made some final adjustments in schemas/README.md.

@webmat
Copy link
Contributor

webmat commented Nov 18, 2020

Alright, thanks for adding these mentions. Perhaps we should say "Beta notices should not..."

@ebeahan
Copy link
Member Author

ebeahan commented Nov 18, 2020

Ok, I made that adjustment.

webmat
webmat previously approved these changes Nov 18, 2020
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ebeahan ebeahan merged commit 584ddbf into elastic:master Nov 18, 2020
@ebeahan ebeahan deleted the add-beta-marker branch November 18, 2020 19:47
ebeahan added a commit to ebeahan/ecs that referenced this pull request Nov 18, 2020
ebeahan added a commit that referenced this pull request Nov 18, 2020
@ebeahan ebeahan removed 1.x needs_backport ready Issues we'd like to address in the future. labels Nov 18, 2020
ebeahan added a commit to ebeahan/ecs that referenced this pull request Dec 3, 2020
dseeley added a commit to dseeley/ecs that referenced this pull request May 5, 2021
* bumping version for 1.x release branch (elastic#921)

* [1.x] add related.hosts (elastic#913) (elastic#924)

* [1.x][DOCS] Fixes SIEM links (elastic#936)

* [1.x] Consolidate field-details doc template (elastic#897) (elastic#946)

* Add http.[request|response].mime_type (elastic#944) (elastic#949)

* [1.x] Cut 1.6 Changelog (elastic#933) (elastic#952) (elastic#953)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add threat.technique.subtechnique (elastic#951) (elastic#956)

Co-authored-by: Ross Wolf <[email protected]>

* [1.x] Nest as for foreign reuse (elastic#960) (elastic#962)

* [1.x] Remove `expected_event_types` from protocol (elastic#964) (elastic#965)

* [1.x] Expand definitions of source and destination field sets (elastic#967) (elastic#973)

* [1.x] Introduce `--strict` flag (elastic#937) (elastic#975)

* [1.x] Add example value composite type checking (elastic#966) (elastic#976)

* Add example value composite type checking (elastic#966)
* generate csv artifact

* [1.x] Add event category configuration (elastic#963) (elastic#977)

* [1.x] Add normalizer multi-field capability (elastic#971) (elastic#978)

Co-authored-by: Eric Beahan <[email protected]>

Co-authored-by: Madison Caldwell <[email protected]>

* [1.x] Add mapping network event guidance doc (elastic#969) (elastic#983)

* [1.x] Removing unneeded link under `Additional Information` (elastic#984) (elastic#985)

* [1.x] Add discrete attribute to field details page headers (elastic#989) (elastic#990)

* [1.x] Uniformity across domain name breakdown fields (elastic#981) (elastic#994)

Co-authored-by: Mathieu Martin <[email protected]>

* Add --oss flag to the ECS generator script (elastic#991) (elastic#995)

* Add network directions ingress and egress (elastic#945) (elastic#997)

* Mention ECS Mapper in the main documentation (elastic#987) (elastic#1000)

Co-authored-by: Dan Roscigno <[email protected]>

* [1.x] Introduce experimental artifacts (elastic#993) (elastic#1001)

Co-authored-by: Mathieu Martin <[email protected]>

* Bump version to 1.8.0-dev in branch 1.x (elastic#1011)

* Cut 1.7 changelog (elastic#1010) (elastic#1012)

* [1.x] Clarify that file extension should exclude the dot. (elastic#1016) (elastic#1020)

* [1.x] Add usage docs section (elastic#988) (elastic#1024)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] feat: include alias path when generating template (elastic#877) (elastic#1035)

Co-authored-by: Richard Gomez <[email protected]>

* [1.x] Add support for `scaling_factor` in the generator (elastic#1042) (elastic#1055)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add fallback for constant_keyword (elastic#1046) (elastic#1056)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add wildcard type support to go code generator (elastic#1050) (elastic#1057)

* add wildcard type support

* also add version and constant_keyword

* changelog

* [1.x] New default make task that generates main and experimental artifacts. (elastic#1041) (elastic#1060)

Also changing the order of the 'generate' task: it now starts with the new generator, then runs the legacy scripts.

* [1.x] Change the index pattern in the sample template. (elastic#1048) (elastic#1068)

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "getting-started" (elastic#1073) (elastic#1079)

Co-authored-by: EamonnTP <[email protected]>

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "products-solutions" page (elastic#1074) (elastic#1083)

Co-authored-by: EamonnTP <[email protected]>

* [1.x] Add event.category session. (elastic#1049) (elastic#1093)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add event.category registry (elastic#1040) (elastic#1094)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add --ref support for experimental artifacts (elastic#1063) (elastic#1101)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Remove experimental event.original definition (elastic#1053) (elastic#1104)

* [1.x] Add missing `process.thread.name` to experimental definitions (elastic#1103) (elastic#1106)

* [1.x] Remove index parameter for wildcard fields (elastic#1115) (elastic#1119)

* [1.x] Add dns.answer object into experimental schema (elastic#1118) (elastic#1121)

* [1.x] Clarify x509 definition guidance for network events with only one cert (elastic#1114) (elastic#1123)

* [1.x] Indicate when artifacts include experimental changes (elastic#1117) (elastic#1125)

* [1.x] Add os.type field, with list of allowed values (elastic#1111) (elastic#1130)

* [1.x] Add support for constant_keyword's 'value' parameter (elastic#1112) (elastic#1132)

* [1.x] Beta label support (elastic#1051) (elastic#1133)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Backport elastic#1134 and elastic#1135 (elastic#1136)

* Remove temporary ifeval in "getting started" page, add link to Metrics docs (elastic#1134)
* Remove temporary ifeval from products page, add link to Metrics (elastic#1135)

* Two small documentation backports (elastic#1149)

* Remove an incorrect `event.type` from the 'converting' page (elastic#1146)
* Mention Logstash support for ECS in the 'products' page (elastic#1147)

* [1.x] Reinforce the exclusion of the leading dot from url.extension (elastic#1151) (elastic#1152)

* [1.x] Make all fields linkable directly via an HTML ID (elastic#1148) (elastic#1154)

* [1.x] Tracing fields should be at the root (elastic#1165)

* Add notice to the tracing field set, about not nesting field names. (elastic#1162)
* Tracing fields should be at top level in Beats artifact (elastic#1164)

* [1.x] Usage of brackets for a URL containing IPv6 address (elastic#1131) (elastic#1168)

* [1.x] 6.x index template data type fallback (elastic#1171) (elastic#1172)

* [1.x] Apply RFC 0007 stage 3 changes - multi-user (elastic#1066) (elastic#1175)

Conflict: deleted file rfcs/text/0007-multiple-users.md as RFCs are not backported to version branches.

* [1.x] Handle `error.stack_trace` case for ES 6.x template (elastic#1176) (elastic#1177)

* [1.x] Add composable index templates artifacts (elastic#1156) (elastic#1179)

* [1.x] Move _meta section back inside mappings, in legacy templates. (elastic#1186) (elastic#1187)

Backports the following commits to 1.x:

* Move _meta section back inside mappings, in legacy templates. (elastic#1186) 

This fixes an issue introduced by elastic#1156, discovered in elastic#1180. Composable templates support `_meta` at the template's root, but legacy templates don't. So we're just putting it back inside the mappings for legacy templates.

This also fixes missing updates to the component template, after the introduction of wildcard in elastic#1098.

* [1.x] Apply the RFC 0005 stage 2 (host metrics) changes in the experimental artifacts (elastic#1159) (elastic#1184)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Stage 3 changes for wildcard RFC 0001 (elastic#1098) (elastic#1183)

* [1.x] Conditional handling in es_template.template_settings (elastic#1191) (elastic#1192)

* [1.x] Artifacts docs page (elastic#1189) (elastic#1195)

* [1.x] Remove beta warning label from categorization fields docs (elastic#1067) (elastic#1196)

* [1.x] Correct wording of `event.reference` description (elastic#1181) (elastic#1197)

* Bump version to 1.9.0-dev in branch 1.x (elastic#1198)

* [1.x] Cut 1.8 FF changelog.next.md elastic#1199 (elastic#1201)

* Merge custom and core multi_fields arrays (elastic#982) (elastic#1213)

Co-authored-by: Jonathan Buttner <[email protected]>

* [1.x] Stage 2 changes for RFC 0009 - data_stream fields (elastic#1215) (elastic#1222)

* [1.x] add http.request.id (elastic#1208) (elastic#1223)

Co-authored-by: Eric Beahan <[email protected]>
Co-authored-by: Gil Raphaelli <[email protected]>

* [1.x] add cloud.service.name (elastic#1204) (elastic#1224)

* add cloud.platform

* expand cloud.platform description

* move to cloud.service.name

Co-authored-by: Gil Raphaelli <[email protected]>

* [1.x] Add ssdeep hash (elastic#1169) (elastic#1227)

Co-authored-by: Andrew Stucki <[email protected]>

* [CI] Switch to GitHub actions (elastic#1236) (elastic#1245)

Co-authored-by: Eric Beahan <[email protected]>

Co-authored-by: Andrew Stucki <[email protected]>

* Revert wildcard adoption back to experimental stage (elastic#1235) (elastic#1243)

* Add scaled_float type to go generator (elastic#1250) (elastic#1251)

* add scaled_float

* changelog

* Add categorization fields usage docs (elastic#1242) (elastic#1257)

* add time_zone, postal_code, and continent_code (elastic#1229) (elastic#1258)

* Specify MAC address format (elastic#456) (elastic#1260)

Co-authored-by: Robin Schneider <[email protected]>

* finalize 1.8.0 changelog (elastic#1262) (elastic#1265)

* Add additional host fields (elastic#1248) (elastic#1267)

Co-authored-by: kaiyan-sheng <[email protected]>

* Stage 1 changes for RFC 0014 - extend pe fields (elastic#1256) (elastic#1270)

* Add 2 fields to code_signature (elastic#1269) (elastic#1272)

Co-authored-by: Yamin Tian <[email protected]>

* Stage 3 changes for RFC 0007 - remove beta attribute (elastic#1271) (elastic#1273)

* Stage 1 experimental changes for RFC 0008 - threat.indicator fields (elastic#1268) (elastic#1274)

* Stage 1 changes for RFC 0015 - add elf fieldset (elastic#1261) (elastic#1275)

* Cut 1.9 FF CHANGELOG.next.md (elastic#1277)

* lock go version in actions (elastic#1283) (elastic#1290)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts (elastic#1310) (elastic#1320)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts

* Bump pyyaml from 5.3b1 to 5.4 in /scripts (elastic#1318) (elastic#1325)

Co-authored-by: Eric Beahan <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Adjust terminology - change whitelist to allowlist (elastic#1315) (elastic#1331)

Co-authored-by: Dominic Page <[email protected]>

* Remove -dev label from 1.9 version (elastic#1329)

* remove -dev label from 1.9 version

* generate artifacts

* removing rules artifacts

* Cut 1.9 changelog (elastic#1328)

* move 1.9 changes to changelog

* add 1.9 release changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants