Skip to content

[kibana] Simplify ECS formatted logs ingest pipelines - #4175

Merged
crespocarlos merged 8 commits into
elastic:mainfrom
crespocarlos:4047-kibana-log-pipeline-review
Sep 26, 2022
Merged

[kibana] Simplify ECS formatted logs ingest pipelines#4175
crespocarlos merged 8 commits into
elastic:mainfrom
crespocarlos:4047-kibana-log-pipeline-review

Conversation

@crespocarlos

@crespocarlos crespocarlos commented Sep 9, 2022

Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #4047

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

elasticmachine commented Sep 9, 2022

Copy link
Copy Markdown

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

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.

@crespocarlos 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
@crespocarlos
crespocarlos marked this pull request as ready for review September 9, 2022 16:12
@crespocarlos
crespocarlos requested a review from a team as a code owner September 9, 2022 16:12
@andrewkroh andrewkroh changed the title Simplify ECS formatted logs ingest pipelines Sep 9, 2022
@matschaffer

Copy link
Copy Markdown
Contributor

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

@crespocarlos

Copy link
Copy Markdown
Contributor Author

I just realized I pointed to a wrong ticket now.

@crespocarlos

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@crespocarlos

Copy link
Copy Markdown
Contributor Author

/test

@elasticmachine

elasticmachine commented Sep 12, 2022

Copy link
Copy Markdown

🌐 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) 💚
server.ssl.certificateAuthorities:
["/usr/share/kibana/config/certs/ca-cert.pem"]

elasticsearch.hosts: ["https://elasticsearch:9200"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

xpack.security.enabled has to be true too.

@matschaffer matschaffer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

## Compatibility

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Screen Shot 2022-09-14 at 14 57 41

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@crespocarlos crespocarlos Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@elasticmachine

elasticmachine commented Sep 15, 2022

Copy link
Copy Markdown

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@matschaffer matschaffer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can do this to have it use the .env value

Suggested change
- "KIBANA_PASSWORD=changeme"
- KIBANA_PASSWORD

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

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 matschaffer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sweet, I got some audit logs this time too!

Screen Shot 2022-09-26 at 13 16 30

LGTM

@matschaffer matschaffer mentioned this pull request Sep 26, 2022
4 tasks
@crespocarlos
crespocarlos merged commit d046216 into elastic:main Sep 26, 2022
@crespocarlos
crespocarlos deleted the 4047-kibana-log-pipeline-review branch September 26, 2022 07:59
orestisfl pushed a commit to orestisfl/integrations that referenced this pull request May 15, 2026
* Simplify ECS formatted logs ingest pipelines

* Add kibana service for testing

* Fix kibana service version

* Trying to simplify package service

* Add audit log generation capability to kibana service

* Use .env vars

* pass ELASTIC_PASSWORD to setup.sh

Co-authored-by: Mat Schaffer <mat@elastic.co>
Co-authored-by: klacabane <kevin.lacabane@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

4 participants