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

Introducing doc section for the expected values in reserved fields #684

Merged
merged 18 commits into from
Dec 12, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Dec 9, 2019

Please note that this PR is not focused on the actual values that belong in these fields. Feedback on those will be requested separately. This PR is solely focused on the structure of the documentation pages and implementing the plumbing driving this.

Preview: http://ecs_684.docs-preview.app.elstc.co/diff

TODO

  • Have these pages populated by the generator, from YAML files
  • The "expected values" mentioned in the field definitions shouldn't contain asciidoc, as these defs are used elsewhere.
  • Current field definition should list all acceptable values and link to the page with details (as proposed in this comment)
  • Document how to populate the "acceptable_values"

Open questions

  • Does the current navigation layout make sense? What can be improved? Yes
  • Do we agree we will list the expected event.type values for each event.category? Yes
  • Should we do the above at other levels as well? (e.g. event.kind?)
    • No obvious need at this time.
  • Should the event.type under a given category come with additional contextual details, when used within that category?
    • No time to write these additional contextualized descriptions for now.
  • Should the expected values (e.g. "authentication") be linkable directly? No immediate need.

Follow-up work

  • Replace the lorem ipsum in these locations
    • Main event.type definition
    • top of category field section
    • top of each categorization field's page
    • description of each value (all values for event.kind, event.category, event.type, event.outcome)
  • Make room for example sections
    • At the bottom of each field page
    • (stretch goal) For each value

cc @MikePaquette @benskelker

@webmat webmat self-assigned this Dec 9, 2019
@benskelker
Copy link
Contributor

@webmat
I'm just trying to understand the scope of this. How many fields, more or less, will need predetermined values?

@MikePaquette
Copy link
Contributor

MikePaquette commented Dec 10, 2019

@benskelker here's a scoping estimate:
Four fields

  1. event.kind: ~6 defined values
  2. event.category: ~10 defined values, growing to 30 over time
  3. event.type: ~12 values
  4. event.outcome: ~2-3 values

@benskelker
Copy link
Contributor

Thanks @MikePaquette
My instinct is to try and keep the fields and their values on the same page (we've already had requests for a single reference page that lists all fields).
How about adding a flat list of allowed values in the current field row, and a link to a section on the same page with more detailed descriptions? We'd need to use complex Asciidoctor tables to make it readable - but it's doable.
@webmat - any thoughts?

@MikePaquette
Copy link
Contributor

@webmat Here is some feedback:

Does the current navigation layout make sense? What can be improved?

I think this layout looks great. See next comment for questions about examples.

Do we agree we will list the expected event.type values for each event.category?

Yes. I think we should draw a distinction between allowed and expected throughout the categorization section. For example, the values that ECS defines for these fields should be called allowed values, as they are the only ones that can be used for ECS compatibility. However, the subset of allowed values of event.type that are thought to be commonly used can be called expected values. It is possible there will be a use case we did not foresee that would call for the use of an "allowed-but-not-expected" value of event.type to be used with a given event.category value, for example.

Should we do the above at other levels as well? (e.g. event.kind?)

👍

Should the event.type under a given category come with additional contextual details, when used within that category?

Sounds like a good idea, but I think the examples section (see next comment) might obviate the need for explicit details in this section

Should the expected values (e.g. "authentication") be linkable directly?

Not sure. Linkable from where?

@MikePaquette
Copy link
Contributor

@webmat I think that detailed examples are going to be important for ECS users who are creating their own custom mappings. Have you given some thought as to where we might put the examples? I see two options:

  1. Inline with the field value definitions.
  • Pro: immediate concept reinforcement of the usage of this field value
  • Con: users need to know the correct field value before seeing the example
  1. In a separate section under ECS Category Field Values
  • Pro: reads more like a reference arrange by general log descriptions: firewall, endpoint, database, etc., then look up the relevant categorization fields in the table
  • Con: delayed concept reinforcement for those reading docs serially.

I would vote for option 2.@

@MikePaquette
Copy link
Contributor

MikePaquette commented Dec 10, 2019

@benskelker re: #684 (comment)

How about adding a flat list of allowed values in the current field row, and a link to a section on the same page with more detailed descriptions? We'd need to use complex Asciidoctor tables to make it readable - but it's doable.

I think this sounds good, but would like to see an example of how it looks. Also, need to consider the examples discussion #684 (comment) as well.

@benskelker
Copy link
Contributor

benskelker commented Dec 10, 2019

There's a few ways we can do this, but something similar to:
Screenshot 2019-12-10 at 15 39 18

We'd then add examples in the dedicated field value section.

@webmat
Copy link
Contributor Author

webmat commented Dec 10, 2019

@MikePaquette Yes, excellent point about making room for examples, thanks for the reminder. How would you envision them? A single "examples" section at the bottom of each page (once at the bottom of event.kind, once at the bottom of event.category, etc)? Or do you think we'll need one per value (e.g. one for category "authentication", one for category "process", etc)?

I'd like to start with one example section per page (total of 4 example sections), because the other alternative requires coming up with 50 or so examples. This is something we would need to build over time, as it's going to require a lot more effort.

In the meantime, I'll see if I can write the code to support both, when examples are available at either level (TBD).

@benskelker I like your suggestion to list each acceptable value directly in the field definition without details. It'll be good for experienced users who just need to remember the exact wording they're looking for. However this listing is not going to be enough, we do need new pages to fully explain this. Here's how each set of values (4 groups of values, for each of the fields) will work:

  • They will have up to 20 expected values
  • Every single value will have a detailed (potentially multiple paragraph) definition
  • Most values would likely need concrete examples, as Mike points out.
  • Some of the values (like category) will detail a narrower set of expected values that accompany it. E.g. event.category "authentication" is only expected to have event.type "start", "end", "info". Note that in total there's also around 20 potential values for event.type, each fitting for different event categories.

I'm not opposed to the field definition actually listing each valid value without details, that actually sounds like a good idea. But we will need the new documentation sections to fully flesh out and document the points above.

@benskelker
Copy link
Contributor

@webmat
I'm not suggesting we shouldn't have in-depth information about the allowed field values. I am trying to work out if we can have all the information users need about a field set on one page.

@webmat
Copy link
Contributor Author

webmat commented Dec 10, 2019

@benskelker Thanks for clarifying, and yes this is a great idea.

If you could come up with the appropriate asciidoc snippet to generate this list, I will incorporate it here.

@benskelker
Copy link
Contributor

benskelker commented Dec 11, 2019

@webmat

If you could come up with the appropriate asciidoc snippet to generate this list, I will incorporate it here.

Sure

@nik9000 is working on transforming docs straight to HTML. This would make our lives easier and should be completely transparent. Is it ok if I ask Nik if we can use direct HTML for the ECS docs?

@webmat
Copy link
Contributor Author

webmat commented Dec 11, 2019

Yeah Nik already opened elastic/docs#1559 but with the time pressure for next week's release, I'm not looking at that until the release is out.

I don't think that's gonna change the format of the asciidoc we're writing though, right? Can you share the snippet that would let me include the list side by side like you demonstrated above? I'd love to include it in the PR ASAP. I think it looks good :-)

@benskelker
Copy link
Contributor

@webmat sent via slack. let me know if you have any problems.

@webmat
Copy link
Contributor Author

webmat commented Dec 12, 2019

Ok, I'm satisfied with the state of the generator to merge. Can I get a sanity check please? :-)

We need to follow up with some additional work on it, but can be done as a separate PR. I've moved these items in the "Follow-up work" section in the issue body.

Merging this PR will let me continue following the plan outlined in meta-issue #691.

@webmat webmat marked this pull request as ready for review December 12, 2019 06:16
Copy link
Contributor

@andrewstucki andrewstucki left a comment

Choose a reason for hiding this comment

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

Wondering if we can cross-link individually specified field expected values in the docs, i.e. in:

Important: The field value must be one of the following:

alert        event

link alert and event each individually to something like event-kind.html#alert and event-kind.html#event Might make the ergonomics a bit easier when navigating rather than clicking on the subsequent link and scrolling down the page to find the value you're looking for, especially as we deal with things like event.category which have more valid expected values to list out on one page.

I'm assuming the above could be implemented in a subsequent PR if you like the idea, so approving.

@webmat
Copy link
Contributor Author

webmat commented Dec 12, 2019

Yeah that's what I meant above, with

Should the expected values (e.g. "authentication") be linkable directly?

I'll keep a note to address later. Not a blocker for 1.4 & initial introduction, but may indeed prove useful, especially if the value descriptions end up being pretty long.

@webmat
Copy link
Contributor Author

webmat commented Dec 12, 2019

Thanks @andrewstucki, I'll merge this soon.

Anybody reviewing this after merge, keep in mind that master is just a dev branch. Any feedback on this is still welcome and up for discussion. I'll address further feedback in the next steps

@webmat webmat merged commit 87aab80 into elastic:master Dec 12, 2019
dcode pushed a commit to dcode/ecs that referenced this pull request Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants