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

RHDEVDOCS-3258 - Add Log Source #42331

Merged
merged 1 commit into from
Jun 17, 2022
Merged

Conversation

libander
Copy link
Contributor

@libander libander commented Feb 23, 2022

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 23, 2022
@netlify
Copy link

netlify bot commented Feb 24, 2022

Deploy Preview for osdocs ready!

Name Link
🔨 Latest commit 7b442e11e9ef0eab47d064a7b10b0a5ca0a7b476
🔍 Latest deploy log https://app.netlify.com/sites/osdocs/deploys/6285359e4b4f1c0008997ea2
😎 Deploy Preview https://deploy-preview-42331--osdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

== Adding log source information to message output

You can add namespace_name, pod_name, and container_name to the `message` field of the record by adding the `AddLogSource` field to your `ClusterLogForwarder` CR.

Copy link
Contributor

@rolfedh rolfedh Feb 24, 2022

Choose a reason for hiding this comment

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

Suggested change
.Procedure
* Add `AddLogSource: true` to your `ClusterLogForwarder` custom resource definition file.
+
.Example `ClusterLogForwarder` CR

Copy link
Contributor

Choose a reason for hiding this comment

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

@libander this seems like an assembly that should have three modules: 1. A procedure module for 'Configuring log forwarding using the syslog protocol' or something on those lines. 2. A procedure module with Rolfe's suggestion above for this section - 'Adding log source information to message output' (procedure module since it starts with a gerund.) and 3. A reference module for Syslog parameters.
Currenlty, this whole section: Forwarding logs using the syslog protocol is marked as content-type: PROCEDURE, which is misleading.
Considering the parent section, I believe, you might need to use nested assemblies under the assembly for the parent section: https://deploy-preview-42331--osdocs.netlify.app/openshift-enterprise/latest/logging/cluster-logging-external.html
While this may be outside the scope of this PR I suggest you file a follow up enhancement Jira issue for this.
However for this Jira, I suggest you implement this recommendation by @rolfedh

@gkarager
Copy link

gkarager commented Mar 3, 2022

@libander , I have verified this PR using Logging 5.2.
Below is the syslog message output with AddLogSource which is not as per the doc:

2022-03-02T15:52:11+00:00 logci2904-svsbv-worker-c-ssdrn.c.openshift-qe.internal mytag: {"docker":{"container_id":"fe7fb4e0a6e3140cbecdff91863eb17d9136db5afa1be4424af6d3407f835e91"},"kubernetes":{"container_name":"centos-logtest","namespace_name":"npzon","pod_name":"centos-logtest-dzjwp","container_image":"quay.io/openshifttest/ocp-logtest@sha256:16232868ba1143721b786dbabb3f7384645acb663fadb4af48e9ea1228a67635","container_image_id":"quay.io/openshifttest/ocp-logtest@sha256:16232868ba1143721b786dbabb3f7384645acb663fadb4af48e9ea1228a67635","pod_id":"3d106242-b12c-4b5f-83e3-99d81b03c305","pod_ip":"10.129.2.77","host":"logci2904-svsbv-worker-c-ssdrn.c.openshift-qe.internal","labels":{"run":"centos-logtest","test":"centos-logtest"},"master_url":"https://kubernetes.default.svc","namespace_id":"a2c56119-1f2b-420f-9b0b-9cff6531e179","namespace_labels":{"kubernetes_io/metadata_name":"npzon"}},"message":"namespace_name=npzon, container_name=centos-logtest, pod_name=centos-logtest-dzjwp, message={\"message\":\"MERGE_JSON_LOG=true\",\"level\":\"debug\",\"Layer1\":\"layer1 0\",\"layer2\":{\"name\":\"Layer2 1\",\"tips\":\"Decide by PRESERVE_JSON_LOG\"},\"StringNumber\":\"10\",\"Number\":10,\"foo.bar\":\"Dot Item\",\"{foobar}\":\"Brace Item\",\"[foobar]\":\"Bracket Item\",\"foo:bar\":\"Colon Item\",\"foo bar\":\"Space Item\"}","level":"unknown","hostname":"logci2904-svsbv-worker-c-ssdrn.c.openshift-qe.internal","pipeline_metadata":{"collector":{"ipaddr4":"10.0.128.3","inputname":"fluent-plugin-systemd","name":"fluentd","received_at":"2022-03-02T15:50:45.549186+00:00","version":"1.7.4 1.6.0"}},"@timestamp":"2022-03-02T15:50:45.547394+00:00","viaq_index_name":"app-write","viaq_msg_id":"Yjc0ZWZmNTYtNGI2MC00MjgwLTk3NTMtZjhlMTQ0MTQ2NTI4","log_type":"application"}

@libander
Copy link
Contributor Author

@libander , I have verified this PR using Logging 5.2. Below is the syslog message output with AddLogSource which is not as per the doc:

2022-03-02T15:52:11+00:00 logci2904-svsbv-worker-c-ssdrn.c.openshift-qe.internal mytag: {"docker":{"container_id":"fe7fb4e0a6e3140cbecdff91863eb17d9136db5afa1be4424af6d3407f835e91"},"kubernetes":{"container_name":"centos-logtest","namespace_name":"npzon","pod_name":"centos-logtest-dzjwp","container_image":"quay.io/openshifttest/ocp-logtest@sha256:16232868ba1143721b786dbabb3f7384645acb663fadb4af48e9ea1228a67635","container_image_id":"quay.io/openshifttest/ocp-logtest@sha256:16232868ba1143721b786dbabb3f7384645acb663fadb4af48e9ea1228a67635","pod_id":"3d106242-b12c-4b5f-83e3-99d81b03c305","pod_ip":"10.129.2.77","host":"logci2904-svsbv-worker-c-ssdrn.c.openshift-qe.internal","labels":{"run":"centos-logtest","test":"centos-logtest"},"master_url":"https://kubernetes.default.svc","namespace_id":"a2c56119-1f2b-420f-9b0b-9cff6531e179","namespace_labels":{"kubernetes_io/metadata_name":"npzon"}},"message":"namespace_name=npzon, container_name=centos-logtest, pod_name=centos-logtest-dzjwp, message={\"message\":\"MERGE_JSON_LOG=true\",\"level\":\"debug\",\"Layer1\":\"layer1 0\",\"layer2\":{\"name\":\"Layer2 1\",\"tips\":\"Decide by PRESERVE_JSON_LOG\"},\"StringNumber\":\"10\",\"Number\":10,\"foo.bar\":\"Dot Item\",\"{foobar}\":\"Brace Item\",\"[foobar]\":\"Bracket Item\",\"foo:bar\":\"Colon Item\",\"foo bar\":\"Space Item\"}","level":"unknown","hostname":"logci2904-svsbv-worker-c-ssdrn.c.openshift-qe.internal","pipeline_metadata":{"collector":{"ipaddr4":"10.0.128.3","inputname":"fluent-plugin-systemd","name":"fluentd","received_at":"2022-03-02T15:50:45.549186+00:00","version":"1.7.4 1.6.0"}},"@timestamp":"2022-03-02T15:50:45.547394+00:00","viaq_index_name":"app-write","viaq_msg_id":"Yjc0ZWZmNTYtNGI2MC00MjgwLTk3NTMtZjhlMTQ0MTQ2NTI4","log_type":"application"}

@gkarager it seems to have the fields specified, is it the order that's a concern? (would this be potentially variable, depending on CR configuration?)

Copy link
Contributor

@JStickler JStickler left a comment

Choose a reason for hiding this comment

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

Couple of small formatting fixes before this is ready to merge.

modules/cluster-logging-collector-log-forward-syslog.adoc Outdated Show resolved Hide resolved
modules/cluster-logging-collector-log-forward-syslog.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@rolfedh rolfedh left a comment

Choose a reason for hiding this comment

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

Still a couple of issues, but pre-approved to expedite merging.

@Preeticp
Copy link
Contributor

@libander please ask for QE/SME LGTM for this PR to be merged.

@libander
Copy link
Contributor Author

@anpingli

@anpingli
Copy link

@gkarager PTAL

@gkarager
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2022
@JStickler JStickler merged commit d35da96 into openshift:main Jun 17, 2022
@JStickler
Copy link
Contributor

/cherry-pick enterprise-4.8

@JStickler
Copy link
Contributor

/cherry-pick enterprise-4.9

@JStickler
Copy link
Contributor

/cherry-pick enterprise-4.10

@JStickler
Copy link
Contributor

/cherry-pick enterprise-4.11

@openshift-cherrypick-robot

@JStickler: new pull request created: #46834

In response to this:

/cherry-pick 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

@JStickler: new pull request created: #46835

In response to this:

/cherry-pick 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

@JStickler: new pull request created: #46836

In response to this:

/cherry-pick enterprise-4.10

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

@JStickler: new pull request created: #46837

In response to this:

/cherry-pick enterprise-4.11

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.

@libander libander deleted the RHDEVDOCS-3258 branch May 2, 2024 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.8 branch/enterprise-4.9 branch/enterprise-4.10 branch/enterprise-4.11 dev-tools Label for all Odo/Pipelines/Helm/Developer Console/Perspective PRs lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants