[kibana] Simplify ECS formatted logs ingest pipelines #4175

crespocarlos · 2022-09-09T14:18:26Z

What does this PR do?

This PR simplifies the logs ingest pipelines, since they are in ECS format, reusing what's has been done for the Platform Observability package

How to test this PR locally

Build the elasticsearch package: cd packages/kibana && elastic-package build
Start a stack: elastic-package stack up -v -d --version 8.5.0-SNAPSHOT
start the elasticsearch service: cd packages/kibana && elastic-package service up -v
Install the Kibana package
- Set audit log path: /tmp/service_logs/audit.log
- Set audit log path: /tmp/service_logs/kibana.log
Check the Logs UI

elasticmachine · 2022-09-09T14:36:21Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-09-21T10:31:43.298+0000
Duration: 13 min 25 sec

Test stats 🧪

Test	Results
Failed	0
Passed	18
Skipped	0
Total	18

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

matschaffer · 2022-09-12T06:21:53Z

Should there be a new issue for this? The "closes" is currently pointing to a merged PR which points to a closed issue.

crespocarlos · 2022-09-12T07:27:16Z

I just realized I pointed to a wrong ticket now.

crespocarlos · 2022-09-12T07:31:03Z

@elasticmachine merge upstream

crespocarlos · 2022-09-12T07:32:59Z

/test

elasticmachine · 2022-09-12T07:46:26Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`2/2`)	💚
Files	100.0% (`4/4`)	💚 2.693
Classes	100.0% (`4/4`)	💚 2.693
Methods	68.421% (`26/38`)	👎 -21.296
Lines	86.667% (`78/90`)	👎 -4.626
Conditionals	100.0% (`0/0`)	💚

matschaffer · 2022-09-12T07:12:24Z

packages/kibana/_dev/deploy/docker/config/kibana.yml

+server.ssl.certificateAuthorities:
+  ["/usr/share/kibana/config/certs/ca-cert.pem"]
+
+elasticsearch.hosts: ["https://elasticsearch:9200"]


This is the stack-up elasticsearch right? I'm wondering if we should have this docker-compose launch both ES and kibana. Maybe even keep it simple and run them in http mode.

For example, do we know if the certs you have here are always going to work? I'd expect new launches of e-p stack up to use new certs.

This is the stack-up elasticsearch right?

yes!

For example, do we know if the certs you have here are always going to work? I'd expect new launches of e-p stack up to use new certs.

If they change the current certs, this will stop working. I had this in mind when I used the certs. But it probably can be simplified. We just need a Kibana instance that creates logs into /tmp/service_logs. This kibana service doesn't need fleet related stuff and could connect to a new ES instance.

This kibana service doesn't need fleet related stuff and could connect to a new ES instance.

Yep, that's exactly what I'm thinking. Just keep the elastic-package service docker compose as bare-bones as we need to generate good sample logs.

I ended up trying this and it seemed to work crespocarlos#2 but I only got server logs. Is there something we should add to produce some audit logs at this stage? or maybe we can leave that until the system test stage.

Hm. Not really. Both logs should be created if enabled in kibana.yml. Let me fix the elastic-package service setup for Kibana package and I'll check why audit logs are eventually not being created.

Actually, audit logs are not generated unless you do some action with the kibana service instance.

xpack.security.enabled has to be true too.

matschaffer

Some review thoughts for you. I could see merging as-is and planning for more work once the test suite is more complete. Hard to validate all the corners manually.

matschaffer · 2022-09-14T05:54:33Z

packages/kibana/data_stream/audit/elasticsearch/ingest_pipeline/default.yml

-    if: "ctx?.user?.name != null"
+  - pipeline:
+      name: '{{ IngestPipeline "pipeline-json" }}'
+      if: |-


Might be nice to align this conditional with https://github.com/elastic/integrations/blob/main/packages/elasticsearch/data_stream/server/elasticsearch/ingest_pipeline/default.yml#L15-L17

This conditional was copied from platform-observability package. It does seem like an overkill

Or if there's a reason for it, that's fine. But it'd be good to be consistent about how we say "this is a json message"

Now looking at both approaches, here we need 4 processors in total to achieve what this does with 1. Besides, on elasticserach pipeline, apparently the document is dropped if message is not a json whereas here, the document is still ingested.

matschaffer · 2022-09-14T05:56:20Z

packages/kibana/data_stream/log/elasticsearch/ingest_pipeline/pipeline-json.yml

+      field: "*"
+      override: true
+  - join:
+      field: error.stack_trace


This got me thinking that it'd be good to have stack trace sample logs from kibana to make sure this is the right behavior. Maybe not for this PR though.

matschaffer · 2022-09-14T05:58:14Z

packages/kibana/docs/README.md

@@ -8,7 +8,7 @@ If the Kibana instance is using a basepath in its URL, you must set the `basepat

 ## Compatibility

-The `kibana` package works with Kibana 6.7.0 and later.
+The `kibana` package works with Kibana 8.5.0 and later.


Guessing this is a problem beyond this PR, but in trying to check the changes I noticed the formatting on these pages is all weird now.

Yeah. I've noticed this too. I'll try to find what's causing it

matschaffer · 2022-09-15T06:04:55Z

packages/kibana/_dev/deploy/docker/config/kibana.yml

@@ -0,0 +1,78 @@
+server.name: kibana


Oh, I think I forgot to mention this (maybe lost in the github review cycle somewhere), but I opened crespocarlos#2 for an idea at simplifying the setup here.

Aha. Thanks. ~~There's only one thing missing there. We need to reset the kibana_system user's password. It will still generate logs without that though.~~ nevermind! It works :). I'll just try to get audit logs there

I had to slightly complicate it in order to get audit logs there.

Kind of surprised audit logging would require use of a particular user, but I guess if it works it works :)

Review: [kibana] Simplify ECS formatted logs ingest pipelines

elasticmachine · 2022-09-15T14:54:46Z

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

matschaffer

A couple visual inspection thoughts, I didn't try to run it today though.

matschaffer · 2022-09-15T22:16:10Z

packages/kibana/_dev/deploy/docker/docker-compose.yml

+    image: "alpine/curl:latest"
+    environment:
+      - "ES_SERVICE_HOST=http://elasticsearch:9200"
+      - "KIBANA_PASSWORD=changeme"


Can do this to have it use the .env value

Suggested change

- "KIBANA_PASSWORD=changeme"

- KIBANA_PASSWORD

good catch.

matschaffer · 2022-09-15T22:16:47Z

packages/kibana/_dev/deploy/docker/scripts/setup.sh

+until curl --request POST $ES_SERVICE_HOST/_security/user/kibana_system/_password \
+  --user elastic:changeme \
+  --header 'Content-Type: application/json' \
+  --data "{\"password\":\"$KIBANA_PASSWORD\"}"


I went looking at how the ES docker docs did this to see if we should copy https://github.com/elastic/elasticsearch/blob/main/docs/reference/setup/install/docker/docker-compose.yml#L55-L56

But I think I like your rendition better :)

klacabane · 2022-09-21T10:28:38Z

It looks like the audit logs are not produced. Maybe that is caused by my local docker version but I got the following logs:

setup.sh:

{"error":{"root_cause":[{"type":"security_exception","reason":"unable to authenticate user [elastic] for REST request [/_security/user/kibana_system/_password]","header":{"WWW-Authenticate":["Basic realm=\"security\" charset=\"UTF-8\"","ApiKey"]}}],"type":"security_exception","reason":"unable to authenticate user [elastic] for REST request [/_security/user/kibana_system/_password]","header":{"WWW-Authenticate":["Basic realm=\"security\" charset=\"UTF-8\"","ApiKey"]}},"status":401}

log-generation.sh: Kibana server is not ready yetGenerating audit logs
kibana: GET / [security_exception]: unable to authenticate user [kibana_system] for REST request [/]

klacabane · 2022-09-21T10:32:05Z

We were not passing the ELASTIC_PASSWORD to the setup container, I've added it in c90afa9

klacabane · 2022-09-21T10:41:07Z

packages/kibana/_dev/deploy/docker/.env

@@ -0,0 +1,3 @@
+ELASTIC_PASSWORD=changeme
+ELASTIC_VERSION=8.5.0-SNAPSHOT


Nice, we should export this variable in logstash/es package as well that'll make our life easier for #4013

Logstash is done and elasticsearch is pending https://github.com/elastic/integrations/pull/4255/files

klacabane · 2022-09-21T10:42:43Z

packages/kibana/data_stream/log/_dev/test/pipeline/test-common-config.yml

@@ -0,0 +1,2 @@
+dynamic_fields:
+  event.ingested: ".*"


We don't have to ignore event.created here ?

No necessarily, because the event.created is copied from @timestamp, and it never changes, so the test won't fail if you don't ignore it.. event.ingested is dynamic.

crespocarlos · 2022-09-23T07:18:47Z

We were not passing the ELASTIC_PASSWORD to the setup container, I've added it in c90afa9

Thanks for spotting it, Kevin. I might have forgotten to push this change.

matschaffer

Sweet, I got some audit logs this time too!

LGTM

Simplify ECS formatted logs ingest pipelines

dee2c06

Add kibana service for testing

45dd519

crespocarlos added Integration:kibana Kibana Team:Infra Monitoring UI - DEPRECATED Label for the Infrastructure Monitoring UI team. - DEPRECATED - Use Team:obs-ux-infra_services v8.5.0 labels Sep 9, 2022

Fix kibana service version

25aac18

crespocarlos marked this pull request as ready for review September 9, 2022 16:12

crespocarlos requested a review from a team as a code owner September 9, 2022 16:12

andrewkroh changed the title ~~Simplify ECS formatted logs ingest pipelines~~ [kibana] Simplify ECS formatted logs ingest pipelines Sep 9, 2022

matschaffer reviewed Sep 12, 2022

View reviewed changes

Trying to simplify package service

41d8569

matschaffer mentioned this pull request Sep 14, 2022

Review: [kibana] Simplify ECS formatted logs ingest pipelines crespocarlos/integrations#2

Merged

matschaffer reviewed Sep 14, 2022

View reviewed changes

matschaffer reviewed Sep 15, 2022

View reviewed changes

crespocarlos and others added 2 commits September 15, 2022 15:03

Merge pull request #2 from matschaffer/4047-kibana-log-pipeline-review

242397c

Review: [kibana] Simplify ECS formatted logs ingest pipelines

Add audit log generation capability to kibana service

6ddca3a

matschaffer reviewed Sep 15, 2022

View reviewed changes

Use .env vars

1db0bb0

pass ELASTIC_PASSWORD to setup.sh

c90afa9

klacabane reviewed Sep 21, 2022

View reviewed changes

matschaffer approved these changes Sep 26, 2022

View reviewed changes

matschaffer mentioned this pull request Sep 26, 2022

Update kibana logo #4298

Merged

4 tasks

crespocarlos merged commit d046216 into elastic:main Sep 26, 2022

crespocarlos deleted the 4047-kibana-log-pipeline-review branch September 26, 2022 07:59

		@@ -0,0 +1,3 @@
		ELASTIC_PASSWORD=changeme
		ELASTIC_VERSION=8.5.0-SNAPSHOT

[kibana] Simplify ECS formatted logs ingest pipelines #4175

[kibana] Simplify ECS formatted logs ingest pipelines #4175

Conversation

crespocarlos commented Sep 9, 2022 • edited Loading

What does this PR do?

How to test this PR locally

elasticmachine commented Sep 9, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

matschaffer commented Sep 12, 2022

crespocarlos commented Sep 12, 2022

crespocarlos commented Sep 12, 2022

crespocarlos commented Sep 12, 2022

elasticmachine commented Sep 12, 2022 • edited Loading

🌐 Coverage report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matschaffer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 15, 2022 • edited Loading

🚀 Benchmarks report

matschaffer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klacabane commented Sep 21, 2022

klacabane commented Sep 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos commented Sep 23, 2022

matschaffer left a comment

Choose a reason for hiding this comment

crespocarlos commented Sep 9, 2022 •

edited

Loading

elasticmachine commented Sep 9, 2022 •

edited

Loading

elasticmachine commented Sep 12, 2022 •

edited

Loading

matschaffer left a comment •

edited

Loading

crespocarlos Sep 15, 2022 •

edited

Loading

elasticmachine commented Sep 15, 2022 •

edited

Loading