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

docs: Finish Application logging docs #2542

Merged
merged 4 commits into from
Jan 27, 2023

Conversation

bmorelli25
Copy link
Member

Summary

This PR is a follow-up to #2452. Its main purpose is to update the Filebeat configuration with additional details and updated configuration options.

@bmorelli25 bmorelli25 added docs Improvements or additions to documentation backport-8.6 Automated backport with mergify labels Jan 24, 2023
@bmorelli25 bmorelli25 requested a review from a team as a code owner January 24, 2023 00:30
@bmorelli25 bmorelli25 self-assigned this Jan 24, 2023
@github-actions
Copy link
Contributor

A documentation preview will be available soon:

Comment on lines -17 to +19
keys_under_root: true
overwrite_keys: true
add_error_key: true
expand_keys: true
overwrite_keys: true <2>
add_error_key: true <3>
expand_keys: true <4>
Copy link
Member Author

@bmorelli25 bmorelli25 Jan 24, 2023

Choose a reason for hiding this comment

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

Removed keys_under_root as new keys go under root by default. This can be changed with target (source), but I didn't think it was necessary to list it here as we're not suggesting changing the default.

Copy link
Member

@SylvainJuge SylvainJuge Jan 24, 2023

Choose a reason for hiding this comment

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

@bmorelli25 do you know which version introduced this change and does it applies to other keys that are injected into the document ?
In the ready-to-use examples that I currently have (initially built for 8.5.3, but now upgraded to 8.6.0):

Copy link
Member Author

@bmorelli25 bmorelli25 Jan 24, 2023

Choose a reason for hiding this comment

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

For nd_json logs, keys_under_root doesn't do anything and never has; a documentation error in the Filebeat docs made it appear necessary. In 8.2, the documentation was updated to show target instead of keys_under_root. If target is not set, fields go to the root of the event by default (source).

For plain text logs, you are correct. fields_under_root is needed to store custom fields as top-level fields in the output document instead of being grouped under a fields sub-dictionary (source).

@SylvainJuge
Copy link
Member

In order to have correlation between log documents and APM services, we need to (at least) set the value of service.name (and maybe few other fields but I don't know yet which ones).
This is required for the following strategies:

  • plain-text with filebeat
  • ECS logging with filebeat:
    • service.name can be set in ECS logging configuration
    • when not set at ECS logging configuration level and deployed with the Java agent, then the Java agent will set service.name with the value from its own configuration.

When using the other strategies (ECS reformatting and Log sending), then the ECS logging library in the agent is configured with the service.name from the agent configuration, hence making it implicit.

Without this, the end-user will only be able to see logs documents in the Logs Stream UI, not in APM.

@SylvainJuge
Copy link
Member

The service.name field is currently the main way to correlate the logs, which is properly described in elastic/kibana#148788 description:

Logs annotated with a matching service.name are displayed . Log lines without service.name will be matched using container.id and then host.name (in that order).

I don't think that we need to document the container.id and host.name fallback as the container.id is very unlikely to be set by the end user, but it can be implicitly provided when using the APM agents.

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

Changes LGTM from an editorial perspective.

@bmorelli25
Copy link
Member Author

Thanks!

For plaintext, I added a step on log correlation. For ECS logging with filebeat, it looks like all of the ECS guides mention log correlation, so I'm not sure we need to add it to these docs.

@SylvainJuge
Copy link
Member

For the plain-text logs, I think that adding the service.name extra field configuration would be relevant here :

While there is a mention of service.name in the "Application logs" section, this is quite far away and for plaintext it will always be required (unlike ECS logging where it can be set either through logger configuration or directly by the APM agent), hence usually not required in the filebeat configuration.

@bmorelli25 bmorelli25 enabled auto-merge (squash) January 27, 2023 22:00
@bmorelli25 bmorelli25 merged commit 47e0adf into elastic:main Jan 27, 2023
@bmorelli25 bmorelli25 deleted the finish-logging-docs branch January 27, 2023 22:13
mergify bot pushed a commit that referenced this pull request Jan 27, 2023
* docs: follow-ups to app logs

* docs: add log correlation info to plaintext

* docs: add service.name to tabbed widget

(cherry picked from commit 47e0adf)
bmorelli25 added a commit that referenced this pull request Jan 27, 2023
* docs: follow-ups to app logs

* docs: add log correlation info to plaintext

* docs: add service.name to tabbed widget

(cherry picked from commit 47e0adf)

Co-authored-by: Brandon Morelli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.6 Automated backport with mergify docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants