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.

Sign up for GitHub

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

Jump to bottom

[logging] Upgrade ECS to 8.0 #113662

Merged
merged 9 commits into from
Oct 6, 2021
Merged

[logging] Upgrade ECS to 8.0 #113662

merged 9 commits into from
Oct 6, 2021

Conversation

lukeelmers
Copy link
Member

@lukeelmers lukeelmers commented Oct 1, 2021 •
edited
Loading

Closes #112175

Summary

I'm splitting the work outlined in #112175 into two parts:

  • Part 1 (merged in [logging] Upgrade ECS to 1.12 #113583): Upgrade ECS types to 1.12.0. Will be backported to 7.x to coincide with the 7.16 stack release.
  • Part 2 (this PR): Upgrade ECS types to make breaking changes for 8.0.0[1]. Will not be backported.

[1] Starting in 8.0, ECS will be aligning their versioning with stack releases, hence the jump from 1.12 to 8.0

Testing

  • There isn't really a great way to test this; it's basically just "did the TypeScript compile?"
  • As ECS 8.0 hasn't been officially released yet, I'm pulling the changes from two places:
    1. The ECS 8.0 breaking changes issue, and
    2. The CHANGELOG.next.md on ECS's current master branch.
  • Fortunately, there are only a couple of changes that affected us, so the PR surface area is rather small. As we get closer to 8.0 FF, I will keep an eye out for any other changes in ECS 8.0.0 so that we can update these accordingly. The ECS team tells me there are no other breaking changes planned, however if there are any new features added between now and FF (unlikely), then we will need to update them here too for consistency.

I'm going to skip adding a breaking changes note here, as I think this will already be covered more broadly by #112305

@lukeelmers lukeelmers added release_note:breaking Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 backport:skip This commit does not require backporting Feature:Logging labels Oct 1, 2021
@lukeelmers lukeelmers requested a review from a team as a code owner October 1, 2021 19:03
@lukeelmers lukeelmers self-assigned this Oct 1, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@lukeelmers lukeelmers added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:breaking labels Oct 1, 2021
@lukeelmers
Copy link
Member Author

@elasticmachine merge upstream

mshustov
mshustov reviewed Oct 5, 2021
View reviewed changes
packages/kbn-logging/src/ecs/agent.ts
@@ -7,7 +7,7 @@
*/

/**
* https://www.elastic.co/guide/en/ecs/1.12/ecs-agent.html
* https://www.elastic.co/guide/en/ecs/master/ecs-agent.html
Copy link
Contributor

Choose a reason for hiding this comment

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

are you going to upgrade it to 8.0 in the next PR? master might be out of sync with the definitions in the Kibana repo, so we should prefer a pinned version in the link

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we will need to update this in a follow-up as the 8.0 docs aren't available until FF, so all we have is master.

As an alternative we could just link now to the 8.0 URLs, which will return a 404 until they are available.

src/core/server/logging/layouts/__snapshots__/json_layout.test.ts.snap Outdated
@@ -1,13 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`\`format()\` correctly formats record. 1`] = `"{\\"ecs\\":{\\"version\\":\\"1.12.0\\"},\\"@timestamp\\":\\"2012-02-01T09:30:22.011-05:00\\",\\"message\\":\\"message-1\\",\\"error\\":{\\"message\\":\\"Some error message\\",\\"type\\":\\"Some error name\\",\\"stack_trace\\":\\"Some error stack\\"},\\"log\\":{\\"level\\":\\"FATAL\\",\\"logger\\":\\"context-1\\"},\\"process\\":{\\"pid\\":5355}}"`;
exports[`\`format()\` correctly formats record. 1`] = `"{\\"ecs\\":{\\"version\\":\\"8.0.0\\"},\\"@timestamp\\":\\"2012-02-01T09:30:22.011-05:00\\",\\"message\\":\\"message-1\\",\\"error\\":{\\"message\\":\\"Some error message\\",\\"type\\":\\"Some error name\\",\\"stack_trace\\":\\"Some error stack\\"},\\"log\\":{\\"level\\":\\"FATAL\\",\\"logger\\":\\"context-1\\"},\\"process\\":{\\"pid\\":5355}}"`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind updating the test suite with ecs.version assertion against a string to avoid test flakiness?

Copy link
Member Author

Choose a reason for hiding this comment

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

I already asserted against the string itself in the next test, however I went ahead and updated these snapshots so they don't contain a version either. That way there's only one test that needs to be changed when the ECS version is bumped.

@lukeelmers
Copy link
Member Author

@elasticmachine merge upstream

@lukeelmers
Copy link
Member Author

@elasticmachine merge upstream

@lukeelmers
Copy link
Member Author

@elasticmachine merge upstream

@lukeelmers lukeelmers enabled auto-merge (squash) October 6, 2021 17:42
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @lukeelmers

@lukeelmers lukeelmers merged commit f030960 into elastic:master Oct 6, 2021
@lukeelmers lukeelmers deleted the feat/upgrade-ecs-8.0 branch October 6, 2021 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mshustov mshustov mshustov approved these changes

Assignees

@lukeelmers lukeelmers

Labels
backport:skip This commit does not require backporting Feature:Logging release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[core.logging] Update ECS typings to 8.0.0
4 participants
@lukeelmers @elasticmachine @kibanamachine @mshustov