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

[logging] Serverless logging config should default to logging to stdout in json layout #152144

Closed
lukeelmers opened this issue Feb 24, 2023 · 12 comments
Assignees
Labels
Feature:Logging Project:Serverless Work as part of the Serverless project for its initial release Team:Operations Team label for Operations Team v9.0.0

Comments

@lukeelmers
Copy link
Member

In the comments on #114968 (comment) it was discussed whether we should switch our default logging configuration for Docker to use json layout, as Docker logs are typically collected via stdout, and our default format for logs to stdout is pattern layout (aka not-ECS).

While this is something worth considering for self-managed, we should definitely take this approach for serverless. Currently we are overriding this config in the kibana-controller, but ultimately it should be added to Kibana's own serverless configs1:

logging:
  appenders:
    console-json:
      type: console
      layout:
        type: json
  root:
    appenders: [console-json]

--

cc @pebrc

Footnotes

  1. These configs aren't yet being picked up by control plane, so changes won't have an immediate affect.

@lukeelmers lukeelmers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Logging Project:Serverless Work as part of the Serverless project for its initial release labels Feb 24, 2023
@elasticmachine
Copy link
Contributor

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

@pgayvallet
Copy link
Contributor

Currently we are overriding this config in the kibana-controller, but ultimately it should be added to Kibana's own serverless configs1:

(TIL we already have a serverless.yaml config file on main, but anyway)

Just to understand a bit better, what are the upsides of having such serverless-specific 'basic' (understand: also usable without restriction on prem) configuration be held in the repo rather than being managed by the controller as it's done today?

@lukeelmers
Copy link
Member Author

(TIL we already have a serverless.yaml config file on main, but anyway)

Yeah, added in #149878

what are the upsides of having such serverless-specific 'basic' (understand: also usable without restriction on prem) configuration be held in the repo rather than being managed by the controller as it's done today?

When this was discussed last week the conclusion was that we want to (for now) put as many of the "static" configs as possible in the kibana repo so that they can be more easily controlled/audited, and also eventually have tests running Kibana CI against any changes to them. Of course, any dynamic settings (i.e. secrets or "knobs" that can be adjusted at the control plane level) would still need to be added somewhere outside of kibana... we still need to discuss how exactly we want that to work.

(understand: also usable without restriction on prem)

Not sure I follow -- we aren't talking about adding any new configurations here, just about where we are locating static configs that will need to apply to serverless deployments.

@lukeelmers
Copy link
Member Author

As I was thinking about this, one downside that does come to mind is that this would be a less optimal DX for folks running Kibana from source. Devs using the serverless configs would get the (less human-readable) json logs to the console by default, unless they took the extra step of overriding this in a .dev.yml file.

@TinaHeiligers
Copy link
Contributor

Devs using the serverless configs would get the (less human-readable) json logs to the console by default, unless they took the extra step of overriding this in a .dev.yml file

I don't see any issue with letting dev's know how to override the standard logging config we want in serverless mode. Having a default is better than a 'free for all' approach or defaulting to something less verbose (a pattern type).

@afharo
Copy link
Member

afharo commented May 24, 2023

@lukeelmers, do you know if this is a requirement for Serverless or Docker images in general?

Just wondering if this is something that can be solved at the kibana-docker level.

@lukeelmers
Copy link
Member Author

@lukeelmers, do you know if this is a requirement for Serverless or Docker images in general?

I don't think this is a requirement for serverless right now. We are working around it for the time being with the configuration that's being applied in kibana-controller.

That said, I do think it's worth trying to align with ES on this in the long term. After talking to @mark-vieira, it sounds like the current behavior on ES docker images is to log in ecs/json by default everywhere (to file and stdout). The current behavior for Kibana images is to default to pattern layout (stdout).

So ideally either we would:

  1. configure Kibana to log json to stdout by default in the docker images, eventually removing our override from kibana-controller,
  2. or ES would default to text layout in stdout, and we would be consistent without doing anything.

@mark-vieira Before we consider changing this on our side, do you think there's a likelihood ES would look to change the behavior instead (since it's arguably a better experience to default to something more human-readable in stdout)?

If we went with (1), we'd probably need to consider this a breaking change on the Kibana side, so this isn't something we could easily update near-term.


(This topic is discussed a bit toward the end of the thread in #114968 -- I'm going to re-tag this for the operations team. If we determine a change is needed we can track it here or in the old issue, otherwise this can be closed if ES is planning to make changes on their end).

@lukeelmers lukeelmers added Team:Operations Team label for Operations Team and removed Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Jun 6, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@mark-vieira
Copy link

@mark-vieira Before we consider changing this on our side, do you think there's a likelihood ES would look to change the behavior instead (since it's arguably a better experience to default to something more human-readable in stdout)?

Hard to say. There are currently no concrete plans but it's certainly a better experience when you don't have a full observability solution available (local development, debugging, etc). That said this is partially configurable in ES. If you set the ES_LOG_STYLE environment variable to file it uses the typical pattern layout for stdout and logs to the logs directory as well as stdout. If you just want the normal pattern layout and don't want the file appender that's currently not possible. AFAIK this is used by ESS, but primarily because filebeat is used for shipping logs to the observability clusters. Whatever Kibana decides to do it'll need to remain compatible with ESS.

@lukeelmers
Copy link
Member Author

Seeing that:

  • there don't seem to be plans to change anything on the ES side;
  • it is acknowledged that defaulting to pattern layout in stdout is a better experience for humans;
  • kibana can be easily configured to log to stdout in json/ecs format anyway;

I am starting to think that maybe we should just leave the behavior as-is instead of pushing to make a breaking change to Kibana's log output format. We could fragment the behavior and try to log to ecs by default in docker only, but I'm not sure it is worth the confusion. Do you have any feelings about this @elastic/kibana-operations ?

@jbudz
Copy link
Member

jbudz commented Apr 26, 2024

I haven't heard anything from Kibana users over the last few years. I think it's fine to leave as is.

@lukeelmers
Copy link
Member Author

Okay, I will go ahead and close this for now then and we can reopen if we have reason to change our minds later.

@lukeelmers lukeelmers closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logging Project:Serverless Work as part of the Serverless project for its initial release Team:Operations Team label for Operations Team v9.0.0
Projects
None yet
Development

No branches or pull requests

8 participants