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.

Sign up for GitHub

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

Jump to bottom

RHDEVDOCS-3217 Tweaks to JSON parsing topics #35248

Merged
merged 1 commit into from Aug 5, 2021
Merged

RHDEVDOCS-3217 Tweaks to JSON parsing topics #35248

merged 1 commit into from Aug 5, 2021

Conversation

rolfedh
Copy link
Contributor

@rolfedh rolfedh commented Aug 5, 2021 •
edited

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 5, 2021
@netlify
Copy link

netlify bot commented Aug 5, 2021 •
edited

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 4dc7ccf

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/610c2edb758f3d00085d82b5

😎 Browse the preview: https://deploy-preview-35248--osdocs.netlify.app

@sichvoge
Copy link

sichvoge commented Aug 5, 2021

Can you also adjust the first paragraph in https://deploy-preview-35248--osdocs.netlify.app/openshift-enterprise/latest/logging/cluster-logging-enabling-json-logging.html#cluster-logging-forwarding-json-logs-to-the-default-elasticsearch_cluster-logging-enabling-json-logging.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 5, 2021
@rolfedh
Copy link
Contributor Author

rolfedh commented Aug 5, 2021

Can you also adjust the first paragraph in https://deploy-preview-35248--osdocs.netlify.app/openshift-enterprise/latest/logging/cluster-logging-enabling-json-logging.html#cluster-logging-forwarding-json-logs-to-the-default-elasticsearch_cluster-logging-enabling-json-logging.

Updated to say "follow schemas"

@rolfedh
Copy link
Contributor Author

rolfedh commented Aug 5, 2021

@sichvoge Thank you for your comments! This helps. Please review, comment, and/or approve.

@rolfedh rolfedh changed the title RHDEVDOCS-3217 Tweaks to JSON logging topics RHDEVDOCS-3217 Tweaks to JSON parsing topics Aug 5, 2021
@sichvoge
Copy link

sichvoge commented Aug 5, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2021
@kabirbhartiRH
Copy link

/lgtm

@bergerhoffer bergerhoffer added this to the Next Release milestone Aug 5, 2021
bergerhoffer
bergerhoffer reviewed Aug 5, 2021
View reviewed changes
modules/cluster-logging-forwarding-json-logs-to-the-default-elasticsearch.adoc
@@ -1,7 +1,14 @@
[id="cluster-logging-forwarding-json-logs-to-the-default-elasticsearch_{context}"]
= Forwarding JSON logs to the Elasticsearch log store

For the Elasticsearch log store that OpenShift Logging manages, you must create a different index for each format in advance if your JSON log entries _have different formats_. Otherwise, forwarding different formats to the same index can cause type conflicts and cardinality problems.
For an Elasticsearch log store, if your JSON log entries _follow different schemas_, configure the `ClusterLogForwarder` custom resource (CR) to group each JSON schema into a single output definition. This way, Elasticsearch uses a separate index for each schema.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the emphasis on "follow different schemas" is necessary. But I see it was similarly done in the previous text, so I won't hold up the PR over it or anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want users to fuss over this procedure if their JSON log entries all follow one schema. I'd welcome suggestions on a better way to word this.

modules/cluster-logging-forwarding-json-logs-to-the-default-elasticsearch.adoc Outdated Show resolved Hide resolved
@bergerhoffer bergerhoffer added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Aug 5, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 5, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2021
@bergerhoffer bergerhoffer merged commit ba90c21 into openshift:main Aug 5, 2021
@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.9

@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.8

@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.7

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 5, 2021
Merged
@openshift-cherrypick-robot
Copy link

@bergerhoffer: new pull request created: #35262

In response to this:

/cherrypick enterprise-4.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 5, 2021
Merged
@openshift-cherrypick-robot
Copy link

@bergerhoffer: new pull request created: #35263

In response to this:

/cherrypick enterprise-4.8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot
Copy link

@bergerhoffer: #35248 failed to apply on top of branch "enterprise-4.7":

Applying: RHDEVDOCS-3217 Tweaks to JSON logging topics
Using index info to reconstruct a base tree...
M	logging/cluster-logging.adoc
Falling back to patching base and 3-way merge...
Auto-merging logging/cluster-logging.adoc
CONFLICT (content): Merge conflict in logging/cluster-logging.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 RHDEVDOCS-3217 Tweaks to JSON logging topics
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick enterprise-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rolfedh rolfedh mentioned this pull request Aug 5, 2021
Merged
rolfedh
rolfedh commented Aug 5, 2021
View reviewed changes
logging/cluster-logging.adoc
@@ -16,7 +16,7 @@ OpenShift Logging aggregates the following types of logs:

* `application` - Container logs generated by user applications running in the cluster, except infrastructure container applications.
* `infrastructure` - Logs generated by infrastructure components running in the cluster and {product-title} nodes, such as journal logs. Infrastructure components are pods that run in the `openshift*`, `kube*`, or `default` projects.
* `audit` - Logs generated by auditd, the node audit system, which are stored in the */var/log/audit/audit.log* file, and the audit logs from the Kubernetes apiserver and the OpenShift apiserver.
* `audit` - Logs generated by auditd, the node audit system, which are stored in the */var/log/audit/audit.log* file, and the audit logs from the Kubernetes apiserver and the OpenShift apiserver.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, I removed an errant space. @bergerhoffer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bergerhoffer bergerhoffer bergerhoffer left review comments

@sichvoge sichvoge sichvoge approved these changes

Assignees

@sichvoge sichvoge

@kabirbhartiRH kabirbhartiRH

Labels
branch/enterprise-4.7 branch/enterprise-4.8 branch/enterprise-4.9 peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
Next Release
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rolfedh @sichvoge @kabirbhartiRH @bergerhoffer @openshift-cherrypick-robot