Skip to content

[Elastic-Agent] Agent push ECS meta to fleet#17894

Merged
michalpristas merged 9 commits intoelastic:masterfrom
michalpristas:agent-push-meta
Apr 22, 2020
Merged

[Elastic-Agent] Agent push ECS meta to fleet#17894
michalpristas merged 9 commits intoelastic:masterfrom
michalpristas:agent-push-meta

Conversation

@michalpristas
Copy link
Copy Markdown
Contributor

@michalpristas michalpristas commented Apr 22, 2020 •
edited
Loading

What does this PR do?

This PR enhances local metadata sent to fleet on checkin and enroll to conform to ecs format.
It still keeps 3 items it was sending before which will be removed when kibana confirms it does not need it.

Why is it important?

Fixes: #14461

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@michalpristas michalpristas self-assigned this Apr 22, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 22, 2020

Pinging @elastic/ingest-management (Team:Ingest Management)

@michalpristas michalpristas requested a review from ph April 22, 2020 09:39
ph
ph approved these changes Apr 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM

x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go
return nil, err
}

// TODO: remove these values when kibana migrates to ECS
Copy link
Copy Markdown
Contributor

@ph ph Apr 22, 2020

Choose a reason for hiding this comment

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

Is there an issue we can link to?

Copy link
Copy Markdown
Contributor Author

@michalpristas michalpristas Apr 22, 2020

Choose a reason for hiding this comment

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

@nchaulet can i remove those and replace with ECS compliant variants?

Copy link
Copy Markdown
Contributor Author

@michalpristas michalpristas Apr 22, 2020

Choose a reason for hiding this comment

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

agent hostname/name is missing on ingest management page so it needs to stay here for now

Copy link
Copy Markdown
Contributor Author

@michalpristas michalpristas Apr 22, 2020

Choose a reason for hiding this comment

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

linked issue elastic/kibana#64173

x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go Outdated

info := sysInfo.Info()

// Agent
Copy link
Copy Markdown
Contributor

@ph ph Apr 22, 2020

Choose a reason for hiding this comment

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

too many spaces :)

x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go
if err != nil {
return nil, err
}

Copy link
Copy Markdown
Contributor

@ph ph Apr 22, 2020

Choose a reason for hiding this comment

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

I am torn here, if we should gracefully recover from that error. What are the impact of this choice, if we return the error it mean the HTTP call will not be made again.

Copy link
Copy Markdown
Contributor Author

@michalpristas michalpristas Apr 22, 2020

Choose a reason for hiding this comment

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

i think that's true. if we dont pack metadata fleet should keep the previous one and we should continue as usual. you;re right that this sould not be breaking

Copy link
Copy Markdown
Contributor Author

@michalpristas michalpristas Apr 22, 2020

Choose a reason for hiding this comment

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

but i will handle this at the api level, as this might be used elsewere as well and we should react accordingly

Copy link
Copy Markdown
Contributor

@ph ph Apr 22, 2020

Choose a reason for hiding this comment

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

Sound good.

@ph
Copy link
Copy Markdown
Contributor

ph commented Apr 22, 2020

LGTM, added a minor question concerning error handling.

@michalpristas michalpristas mentioned this pull request Apr 22, 2020
Closed
@michalpristas michalpristas merged commit 757e7d1 into elastic:master Apr 22, 2020
michalpristas added a commit to michalpristas/beats that referenced this pull request Apr 23, 2020
@michalpristas
[Elastic-Agent] Agent push ECS meta to fleet (elastic#17894)
4a60a04 
[Elastic-Agent] Agent push ECS meta to fleet (elastic#17894)
@michalpristas michalpristas mentioned this pull request Apr 23, 2020
Merged
6 tasks
michalpristas added a commit that referenced this pull request May 4, 2020
@michalpristas
Cherry-pick #17894 to 7.x: Agent push ECS meta to fleet (#17921)
a1ee6fd 
* [Elastic-Agent] Agent push ECS meta to fleet (#17894)

[Elastic-Agent] Agent push ECS meta to fleet (#17894)

* [Elastic-Agent] ECS compliant Elastic agent metadata sent to fleet  (#18006)

* use meta object

* changelog

* agent.* => elastic.agent.*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ph ph ph approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@michalpristas michalpristas

Labels

enhancement needs_backport PR is waiting to be backported to other branches. review [zube]: In Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Agent] Standardized local metadata

3 participants

@michalpristas @elasticmachine @ph