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

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

Merged
merged 9 commits into from
Apr 22, 2020
Merged

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

merged 9 commits into from
Apr 22, 2020

Conversation

michalpristas
Copy link
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
Collaborator

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
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
Contributor

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
Contributor Author

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
Contributor Author

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
Contributor Author

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
Contributor

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
Contributor

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
Contributor Author

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
Contributor Author

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
Contributor

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
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

@ph ph ph approved these changes

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