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

Change Fleet Server over to use ECS logger to ensure logging format and ECS compliance #371

Closed
EricDavisX opened this issue May 18, 2021 · 10 comments · Fixed by #806
Closed
Assignees
Labels
Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.0.0

Comments

@EricDavisX
Copy link
Contributor

From the below comment:
#358 (comment)

we see that Fleet sever had a bug where 2 fields were not ECS compliant. Instead of attempting to test / validate that the current logging is compliant, we can instead switch to the Elastic ECS logger.

This is a ticket for that switch-over work.

More info about ECS logging here: https://www.elastic.co/guide/en/ecs-logging/overview/master/intro.html

The APM team owns the ECS logging libraries, fyi.

@EricDavisX EricDavisX added Team:Elastic-Agent Label for the Agent team v7.14.0 labels May 18, 2021
@simitt
Copy link

simitt commented May 18, 2021

For go we only support zap and logrus at the moment.

@urso
Copy link

urso commented Jun 2, 2021

Beats/Agent use zap. I have no strong feelings regarding zap vs. zerolog, but given that we have better support for zap and use it in other code bases as well, I think we should standardize Fleet Server on zap as well and start using https://github.com/elastic/ecs-logging-go-zap.

@scunningham Any objections?

@scunningham
Copy link

In my opinion, zerolog is a better logger implementation, both from an API and a performance perspective. Our time is better spent fleshing out ECS support for zerolog rather than convert to zap.

I've already spent some time making the fleet-server logging ECS compliant. Much of the work has to do with changing the field names and has little to do with the logger itself. I don't think we'd be buying much by switching to zap frankly. That said, if somebody wants to rewrite all the logging code, more power to them.

@simitt
Copy link

simitt commented Jun 2, 2021

There is an open issue for adding a formatter to zerolog similar to what we did for logrus and zap. I don't think there are any immediate plans to work on that, but if anyone had the bandwith to contribute, that would be more than welcome.

@urso
Copy link

urso commented Jun 4, 2021

@michel-laterman
Copy link
Contributor

elastic/ecs-logging-go-zerolog has been released at v0.1.0, it's available as

import "go.elastic.co/ecszerolog"

@EricDavisX
Copy link
Contributor Author

I'll update this to 7.15 label for now, I don't see it getting any more action during 7.14. We can re-review later.

@urso
Copy link

urso commented Jul 30, 2021

@michel-laterman @scunningham Didn't we introduce ecszerolog in 7.14 already? Can we close this issue?

@scunningham
Copy link

@urso Work to switch to ecszerolog has not been done.

@jlind23 jlind23 added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label Sep 29, 2021
@andresrc andresrc removed the Team:Elastic-Agent Label for the Agent team label Oct 14, 2021
@Mpdreamz
Copy link
Member

@ruflin @jlind23 similar too: elastic/beats#15544 can we ensure this makes 8.0.0?
Would be a fantastic quick win.

@lykkin lykkin mentioned this issue Oct 25, 2021
5 tasks
@jlind23 jlind23 added backport-v8.0.0 Automated backport with mergify v8.0.0 and removed 8.0-candidate backport-v8.0.0 Automated backport with mergify labels Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.0.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants