-
Notifications
You must be signed in to change notification settings - Fork 162
Update "Ingest logs from a Python application using Filebeat” #1965
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
Update "Ingest logs from a Python application using Filebeat” #1965
Conversation
🔍 Preview links for changed docs: 🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
1f8ff71
to
a7de836
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass — nice work!
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Outdated
Show resolved
Hide resolved
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Outdated
Show resolved
Hide resolved
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Show resolved
Hide resolved
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Outdated
Show resolved
Hide resolved
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Outdated
Show resolved
Hide resolved
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Outdated
Show resolved
Hide resolved
I like (a lot!) and agree with how we are updating and improving this document, but I'd like to share some high level thoughts about it.
IMO, the step The next step If we want to make the doc deployment agnostic (or almost agnostic), the filebeat configuration section should also mention that if the destination is not Elastic Cloud or ECE, the user should use the actual URL of the Elasicsearch cluster instead of https://www.elastic.co/docs/reference/beats/filebeat/configure-cloud-id -> for ECH and ECE only cc: @georgewallace : I think this type of feedback is related with your suggestions about reviews, right? trying to make the new IA and guides more consistent with the new approach. |
@eedugon thank you, that’s some really good feedback! I appreciate it! I agree that the gist of I also agree with your suggestion to rename I think you’re right about making this doc deployment-agnostic, but I’m also wary of how this may impact the others steps in this guide. For example, the difference in authentication, as you mentioned, or the possible UI differences (e.g., in the menu structure) which could impact the accuracy of the detailed steps for creating visualizations... What I like in the guide’s current form is that it describes a workflow from start to finish, and you can actually see a working example of Filebeat for log ingest - whereas if you detach it from a specific deployment, it gets much harder for the reader to troubleshoot possible hiccups that may arise from their specific deployment context. So making this deployment-agnostic would require testing the steps in multiple contexts to make sure they are accurate for all deployments. If @shainaraskas and @colleenmcginnis agree that we should make this doc deployment-agnostic, we could create a follow-up issue for that purpose specifically, and work on it separately. What do you think? That said, I really like the idea of adding a note that this doc uses ECH as the Elastic Stack destination but that it could be adapted to other deployment types. I’m going to add this to the doc. |
aca7e7e
to
5b060a3
Compare
@vishaangelova , I'd say there's a middle point between making a guide like this completely deployment agnostic vs being written only with ECH deployments in mind. Originally this doc was a We have migrated thousands of documents that had a very narrowed scope to the new IA, and in a lot of cases we ended up with leftovers like this. We need to improve this type of issue whenever we start improving and giving some love to documents, as with the new IA in mind the And I'm not saying to make it deployment agnostic right away, but if the guide is not really related with ECH and doesn't strictly need a ECH deployment we should just mention that at the beginning of the document, with some kind of note saying that the guide uses a ECH deployment as the destination of the data but it could be adapted to any other deployment type. That would be at least the I would say that we shouldn't move forward with improving this document without considering that, as this document feels wrong in the new IA (of course it's not a problem of this PR, it was the original doc itself). Just a minor note would at least make it As a separate topic (not deployment types related), I'm also missing in the doc maybe some useful glue / links to:
@shainaraskas , I'd like to hear your thoughts if possible, because this doc feels like the old end to end guides that were duplicating content in certain sections. For these docs to evolve better I think they should be better linked (to and from) with the rest of the documentation. |
Another possibility, we merge this PR as it is, and admin-docs / control-plane docs (so myself for example :) ) could take care of working on the deployment type side of things + adding links to other parts of the deploy and manage docs when needed. Whatever we feel! |
I’ve added a note at the beginning that the steps in this guide could be adapted for other deployments, but I kept the references to ECH for now. Let’s see what the others say about how to proceed with removing those. I’ve also restructured the doc a bit to reflect the observations in your first comment. You are right about the cross-links, I can add references to the docs you mentioned. |
e96913f
to
ee9f608
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an SME, but writeup LGTM!
@vishaangelova , I think it looks very nice now! I'll add some minor comments in other parts trying to enrich part of the content with useful links and alternatives (cloud ID for example). |
closing the loop because I was tagged: I agree that adding a note about the tutorial being written for ECH but being valid with small changes for other deployment types is a great temporary patch. I think adding an item in our backlog to ultimately make this tutorial generic is a good idea - they might be low priority, but they're good to track. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I've added some minor comments suggesting some extra links and clarifications that would help for other deployment types.
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Outdated
Show resolved
Hide resolved
...ngest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md
Show resolved
Hide resolved
4fe9fe1
to
2a048db
Compare
2a048db
to
0a5ed7b
Compare
In this PR:
log
input type with thefilestream
input type and adjusted the config options accordinglyCloses #1640 and #125444
Preview
manage-data/ingest/ingesting-data-from-applications/ingest-logs-from-python-application-using-filebeat.md