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

Migrate container input to use filestream instead of log input #34393

Open
rdner opened this issue Jan 25, 2023 · 6 comments
Open

Migrate container input to use filestream instead of log input #34393

rdner opened this issue Jan 25, 2023 · 6 comments
Assignees
Labels
Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Comments

@rdner
Copy link
Member

rdner commented Jan 25, 2023

Describe the enhancement:

The log input is deprecated and the goal is to unbind the rest of the codebase from it, so we can easily remove it in a future major version. Currently, the container input just wraps the log input and introduces some additional behaviour.

func NewInput(
cfg *conf.C,
outletFactory channel.Connector,
context input.Context,
) (input.Input, error) {
// Wrap log input with custom docker settings
config := defaultConfig
if err := cfg.Unpack(&config); err != nil {
return nil, errors.Wrap(err, "reading container input config")
}
err := cfg.Merge(mapstr.M{
"docker-json.partial": true,
"docker-json.cri_flags": true,
// Allow stream selection (stdout/stderr/all)
"docker-json.stream": config.Stream,
// Select file format (auto/cri/docker)
"docker-json.format": config.Format,
// Set symlinks to true as CRI-O paths could point to symlinks instead of the actual path.
"symlinks": true,
})
if err != nil {
return nil, errors.Wrap(err, "update input config")
}
// Add stream to meta to ensure different state per stream
if config.Stream != "all" {
if context.Meta == nil {
context.Meta = map[string]string{}
}
context.Meta["stream"] = config.Stream
}
return log.NewInput(cfg, outletFactory, context)
}

It might be as easy as replacing the input configuration (in-memory) with a filestream preset made for container logs. Sort of an in-memory integration.

Some things to be aware of:

  1. After introducing the take_over mode in Add the take_over mode for filestream inputs #34292 it should be possible to migrate the persisted state (registry). However, we need to account for what's described in 2.

  2. The container input introduces a new meta data value stream into the persisted state. We need to propagate this value into the filestream state and make sure it's present in the output (as it does for the container input.

  3. log input and filestream are implemented using 2 different plugin systems:

func init() {
err := input.Register("log", NewInput)
if err != nil {
panic(err)
}
}

versus

func Plugin(log *logp.Logger, store loginp.StateStore) input.Plugin {
return input.Plugin{
Name: pluginName,
Stability: feature.Stable,
Deprecated: false,
Info: "filestream input",
Doc: "The filestream input collects logs from the local filestream service",
Manager: &loginp.InputManager{
Logger: log,
StateStore: store,
Type: pluginName,
Configure: configure,
},
}
}

so, filestream cannot be a drop-in replacement for the log input code in the container implementation.

@rdner rdner added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jan 25, 2023
@rdner rdner self-assigned this Jan 25, 2023
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@ChrsMark
Copy link
Member

Hey @rdner I see that #34354 was closed in honor of this. Do we have an estimation of when this current issue will be addressed? @elastic/obs-cloudnative-monitoring team is getting requests for supporting filestream input in hints based autodiscovery and apparently we are still using the old container input as the base config when we are generating filebeat's configurations through hints. See

So in that regard I wonder if it would be easier to deal with #34354 directly in case this migration is going to take longer.

In a more technical detail I wonder if this migration will take care of the input specifics. For example currently we support the co.elastic.logs/json.* setting which is tightly coupled with the container/log input and I wonder how this will be handled when we migrate the container input to use the filestream input under the hood. To my mind this will break things?
Instead of this we can directly manage the input type on hints level and allow the respective input-specific supported settings accordingly. That's why I would prefer handling this one within it's own issue since it looks quite "hints-specific" issue.
Let me know what you think :).

@gizas btw that's quite related to the hints' evaluation work you are doing.

cc: @mlunadia @tommyers-elastic @bturquet

@rdner
Copy link
Member Author

rdner commented Jun 23, 2023

@ChrsMark it's better to ask @pierrehilbert
At some point I even started working on this but it got de-prioritised and I had to switch to something else.

@pierrehilbert
Copy link
Collaborator

Hey @ChrsMark,
I'm expecting us to be able to go back on this topic in August.
We are currently trying to put in place new integration and E2E tests and fix some issue first before moving again on this enhancement.

@ChrsMark
Copy link
Member

Thanks for the update @pierrehilbert @rdner! In any case I believe it would be safer to "fix" hints at first place at #34354. As I mentioned at #34393 (comment) some hints are input specific so I expect that we need to revisit the logic in any case there.
@gizas @tommyers-elastic @mlunadia I think we should prioritize this in the product's backlog.

@gizas
Copy link
Contributor

gizas commented Jul 3, 2023

Created #35984 to specify next steps here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

No branches or pull requests

5 participants