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

[Agent] Inject index with sane defaults to configuration #16328

Closed

Conversation

michalpristas
Copy link
Contributor

Migrated from: #15852
Build on top of: #15809

Added one extra rule for injecting index into inputs based on provided type and detected namespace/dataset type.

Other option was to use decorator but at the time of decorating there are already some pieces of config missing.

Draft until final configuration format is settled and mentioned PR is in, but READY for review.

It overwrites index specified in input.

Fixes: #15690
Fixes: #15691

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest (Project:fleet)

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.

The generated index doesn't use the right format, elastic/kibana#56132

I have added a question for @ruflin, I am not sure what is the behavior we should have if the fields namespace or the dataset is missing. We use default values or we fail some validation?

output:
elasticsearch:
hosts:
- 127.0.0.1:9200
- 127.0.0.1:9300
username: elastic
password: changeme
api_key: TiNAGG4BaaMdaH1tRfuU:KnR6yE41RrSowb0kQ0HWoA
ca_sha256: 7HIpactkIAq2Y49orFOOQKurWxmmSFZhBCoQYcRhJ3Y=
Copy link
Contributor

Choose a reason for hiding this comment

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

Space looks weird here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does indeed

@@ -4,10 +4,13 @@ filebeat:
paths:
- /var/log/hello1.log
- /var/log/hello2.log
index: filebeat-generic-default
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect, it should be:

logs-{dataset}-{namespace}

@ruflin When no dataset and namespace are defined what are the default values for them? The elastic/kibana#56132 doesn't mention it?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this issue mentions defaults #15691
for NS it is default for dataset generic so this follows logs-dataset-namespace correctly

Copy link
Member

Choose a reason for hiding this comment

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

The above should be logs-generic-default.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe I've read it too fast. thanks

- module: apache
setting: two
metricsets: [info]
index: metricbeat-generic-testing
Copy link
Contributor

Choose a reason for hiding this comment

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

should be metrics-{dataset}-{namespace}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

namespace==testing in this case so i think it is correct.

fleet:
kibana_url: https://kibana.mydomain.com:5601
ca_hash: 7HIpactkIAq2Y49orFOOQKurWxmmSFZhBCoQYcRhJ3Y=
checkin_interval: 5m
Copy link
Contributor

@ph ph Feb 17, 2020

Choose a reason for hiding this comment

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

Something weird with the same space in a few place, I think YAML will strip it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is weird i have a single space here

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.

small changes concerning the type for metricbeat related input.

- enabled: false
dataset: sample.error
paths: /var/log/sample/error.log
- type: metrics/system
Copy link
Contributor

Choose a reason for hiding this comment

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

// Apply injects index into input.
func (r *InjectIndexRule) Apply(ast *AST) error {
const defaultNamespace = "default"
const defaultDataset = "generic"
Copy link
Contributor

Choose a reason for hiding this comment

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

++ to have theses constants in the function.

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, we have to synchronize with Kibana changes.

@michalpristas
Copy link
Contributor Author

going to test with kibana, either with working UI or manual API calls

@michalpristas
Copy link
Contributor Author

@ruflin @ph tested with kibana ok, i will create a separate PR targeting master

@michalpristas
Copy link
Contributor Author

created new PR against master here: #16903
closing this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants