-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[Hold review] OBSDOCS-1170 - Created a PR for verification of the CLF content #78477
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
Conversation
* Indicates management state (managed or unmanaged) | ||
// Needs engineering eval re applicability to 6.0 | ||
include::modules/log-forwarding-implementations.adoc[leveloffset=+1] |
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.
@anpingli / @jcantrill : Could you help me understand if this module can be used for 6.0 & if not what changes are needed?
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.
How can I comment on modules/log-forwarding-implementations.adoc?
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.
As we discussed, there are no changes needed for modules/log-forwarding-implementations.adoc. Do you have anything else that you would like to suggest for this module?
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.
https://github.com/openshift/openshift-docs/blob/a199424efd582d3fb7d40c267744e285bd359480/modules/log-forwarding-implementations.adoc
@smunje1 IMO ⬆️ is no longer applicable since Logging 6.0 has no concept of legacy vs multi. All deployments are multi and will be deployed under the same rules; there is no special handling
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.
I am planning to remove the Log forwarding implementations, Legacy implementation sections, and rename Multi log forwarder feature -> Creating a log forwarder. I also modified Creating a log forwarder section.
Please let me know if you have other thoughts. Thanks!
[id="log6x-CLF-tuning_{context}"] | ||
== Output tuning | ||
//need to validate how much of this applies to 6.0 & edit as needed. | ||
include::modules/logging-delivery-tuning.adoc[leveloffset=+1] |
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.
@anpingli / @jcantrill : Could you help me understand if this module can be used for 6.0 & if not what changes are needed?
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.
Output tuning is functionally the same as in 5.9 though it has moved from common to outputs to each individual spec. This was done because not all outputs support the same options
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.
AtLeastOnce is atLeastOnce in 6.-0
AtMostOnce is atMostOnce in 6.0
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.
May be we need to update ClusterLogForwarder CR tuning options as follows?
apiVersion: "observability.openshift.io/v1"
kind: ClusterLogForwarder
metadata:
# ...
spec:
outputs:
tuning:
delivery: atLeastOnce
maxWrite: <integer>
compression: none
minRetryDuration: 1s
maxRetryDuration: 1s
# ...
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.
apiVersion: "observability.openshift.io/v1"
kind: ClusterLogForwarder
metadata:
# ...
spec:
outputs:
- name : <output-name>
type: <output_type>
<output_type>:
tuning:
delivery: atLeastOnce
maxWrite: <integer>
compression: none
minRetryDuration: 1s
maxRetryDuration: 1s
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.
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.
This problem in the current documentation is reflected in documentation bug https://issues.redhat.com/browse/OBSDOCS-1049
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.
Tagging @libander so that we can cover this as part of OBSDOCS-1169.
openshiftLabels:: Adds OpenShift-specific labels to log messages. | ||
|
||
// All of these need to be validated by engineering for 6.0. | ||
include::modules/logging-audit-log-filtering.adoc[leveloffset=+1] |
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.
@anpingli / @jcantrill : Could you help me understand if this module can be used for 6.0 & if not what changes are needed?
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.
This should be usable with the exception of needing to modify the CLF where it is contained
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.
As discussed with @anpingli , there are no changes needed for Overview of API audit filter section. I just updated the apiVersion
and metadata
in Example audit policy.
@jcantrill Let me know if you have other thoughts.
// All of these need to be validated by engineering for 6.0. | ||
include::modules/logging-audit-log-filtering.adoc[leveloffset=+1] | ||
|
||
include::modules/logging-content-filter-drop-records.adoc[leveloffset=+1] |
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.
@anpingli / @jcantrill : Could you help me understand if this module can be used for 6.0 & if not what changes are needed?
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.
This should be usable with the exception of needing to modify the CLF where it is contained
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.
As discussed with @anpingli , there are no changes needed for Configuring content filters to drop unwanted log records section. I just updated the apiVersion
in Example ClusterLogForwarder CR and Additional examples.
@jcantrill Let me know if you have other thoughts.
|
||
include::modules/logging-content-filter-drop-records.adoc[leveloffset=+1] | ||
|
||
include::modules/logging-content-filter-prune-records.adoc[leveloffset=+1] |
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.
@anpingli / @jcantrill : Could you help me understand if this module can be used for 6.0 & if not what changes are needed?
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.
This should be usable with the exception of needing to modify the CLF where it is contained. Maybe it makes sense to remove the CLF envelope and only showcase the filter spec
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.
As discussed with @anpingli , there are no changes needed for Configuring content filters to prune log records section. I just updated the apiVersion
in Example ClusterLogForwarder CR.
@jcantrill Let me know if you have other thoughts.
// This needs to be called out extensively. | ||
[source,yaml] | ||
---- | ||
apiVersion: observability.openshift.io/v1 |
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.
To what extent is this intend to represent a valid sample. As-is, this is not valid and does not properly describe a CLF?
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.
@anpingli could you help with your input here? Thank you!
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.
@anpingli Could you help verify if the following sample properly describe CLF and if it is valid ? If not, could you help with your input?
apiVersion: observability.openshift.io/v1
kind: ClusterLogForwarder
metadata:
name: collector
namespace: openshift-logging
spec:
collector:
nodeSelector:
logcollect: run
resources: {}
outputs:
- name: lokistack
lokiStack:
authentication:
token:
from: secret
secret:
key: token
name: secret-to-lokistack
labelKeys:
- .hostname
- .log_type
- .kubernetes_container_name
- .kubernetes_namespace_name
- .kubernetes_pod_name
target:
name: lokistack-sample
namespace: openshift-logging
tls:
ca:
key: ca-bundle.crt
secret:
name: secret-to-lokistack
type: lokiStack
pipelines:
- name: default-before
inputRefs:
- infrastructure
- application
outputRefs:
- lokistack
serviceAccount:
name: logcollector
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.
Can we use this ClusterLogForwarer CustomResourceDefinition and explain each filed?
https://github.com/openshift/enhancements/blob/master/enhancements/cluster-logging/logs-observability-openshift-io-apis.md#clusterlogforwarer-customresourcedefinition
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.
@anpingli @@r2d2rnd @jcantrill Could you please confirm if the ClusterLogForwarer CustomResourceDefinition can be used as is in our documentation? If not, could you share the validated (working fine) CLF that we can use for our documentation?
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.
This example should be modified. As bug opened, we should set the examples with limits for the collector and also recommending always setting them. The bug is https://issues.redhat.com/browse/OBSDOCS-856.
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.
@anpingli @@r2d2rnd @jcantrill Could you please confirm if the ClusterLogForwarer CustomResourceDefinition can be used as is in our documentation? If not, could you share the validated (working fine) CLF that we can use for our documentation?
@smunje1 This is the enhancement document that was the initial proposal but the actual API has subtly evolved since this example. It is good source for an "overview" but the actual API is the final source of truth. This api reference is generated from the API and @libander has been including it in the docs. Additionally, we are working to provide samples that can be used. @cahartma can you please point to relevent GISTs for the time being
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.
Thanks for the context.
Additionally, we are working to provide samples that can be used.
Can these samples be used as is i.e. Copy + Paste the samples that will be provided for our documentation?
[id="log6x-CLF-tuning_{context}"] | ||
== Output tuning | ||
//need to validate how much of this applies to 6.0 & edit as needed. | ||
include::modules/logging-delivery-tuning.adoc[leveloffset=+1] |
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.
Output tuning is functionally the same as in 5.9 though it has moved from common to outputs to each individual spec. This was done because not all outputs support the same options
The following filters are available: | ||
// Should be an include for each, create: modules/log6<filtername>.adoc - Associated JIRA? | ||
detectMultilineException:: Detects and handles multiline exceptions. | ||
parse:: Parses log messages using Vector's parsing language. |
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.
Parses log messages into the format defined by the filter
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.
Tagging @libander so that we can cover this as part of OBSDOCS-1169.
[id="log6x-CLF-filters_{context}"] | ||
== Filtering logs | ||
|
||
The following filters are available: |
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.
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.
openShiftLabels,detectMultilineException, parse are the properties under filters in 6.0.
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.
Tagging @libander so that we can cover this as part of OBSDOCS-1169.
openshiftLabels:: Adds OpenShift-specific labels to log messages. | ||
|
||
// All of these need to be validated by engineering for 6.0. | ||
include::modules/logging-audit-log-filtering.adoc[leveloffset=+1] |
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.
This should be usable with the exception of needing to modify the CLF where it is contained
// All of these need to be validated by engineering for 6.0. | ||
include::modules/logging-audit-log-filtering.adoc[leveloffset=+1] | ||
|
||
include::modules/logging-content-filter-drop-records.adoc[leveloffset=+1] |
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.
This should be usable with the exception of needing to modify the CLF where it is contained
|
||
include::modules/logging-content-filter-drop-records.adoc[leveloffset=+1] | ||
|
||
include::modules/logging-content-filter-prune-records.adoc[leveloffset=+1] |
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.
This should be usable with the exception of needing to modify the CLF where it is contained. Maybe it makes sense to remove the CLF envelope and only showcase the filter spec
[id="log6x-CLF-samples_{context}"] | ||
== Use case samples | ||
// Should be an include, create: modules/log6x-use-multi-instance.adoc - Associated JIRA? | ||
* Multiple-Instance Mode |
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.
For 6.0 we should be able to drop references to "multi log forwarder" since the intent is there is no special handling or special named instances
* Multiple-Instance Mode | ||
|
||
// Should be an include, create: modules/log6x-use-metric-export.adoc - Associated JIRA? | ||
* Log File Metric Exporter as a Separate Deployment No newline at end of file |
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.
This is still a feature. If users wish to see the metrics produced by this component then they will need to deploy it
@smunje1 QE will keep updating https://docs.google.com/document/d/1qysR-ph64d8jq5sdHSsy4V5pQ_qOpBOdq2bsP3T-Jog/edit. If you refer the example in it. Please let me know if you want to provide/verify some content. |
/assign @anpingli |
|
||
include::modules/logging-input-spec-filter-namespace-container.adoc[leveloffset=+1] | ||
|
||
include::modules/logging-multiline-except.adoc[leveloffset=+1] |
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.
detectMultilineException is an item under filter in 6.0
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.
Maybe the example can be updated like?
apiVersion: "observability.openshift.io/v1"
kind: ClusterLogForwarder
metadata:
name: <log_forwarder_name>
namespace: <log_forwarder_namespace>
spec:
filters:
- name: <name>
type: detectMultilineException
pipelines:
- inputRefs:
- <input-name>
name: <pipeline-name>
filterRefs:
- <filter-name>
outputRefs:
- <output-name>
toc::[] | ||
|
||
The `ClusterLogForwarder` CR serves as a single point of configuration for log forwarding, making it easier to manage and maintain log collection and forwarding rules. | ||
|
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.
we can place pods on specific node or limit resources using .spec.collector.
For example:
spec:
collector:
nodeSelector:
kubernetes.io/os: "linux"
resources: {}
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.
Do we need to add the above information in Configuring log forwarding section?
(Need clarity because this is just the introductory section.)
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.
The above information will be covered in Multi log forwarder CLF and it's callouts.
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.
Multi log forwarder CLF
For 6.0 we should reconsider the use of "multi log forwarder CLF" as starting in 6.0. CLF becomes like any other custom resource on the cluster in that there can always be "multiple". There is no need to distinguish it separately from our legacy usecases
As commented in Jira: OBSDOCS-1169, this PR is now only for OBSDOCS-1170. So, I am changing the PR title and it`s description. |
Loki is a horizontally scalable, highly available, multi-tenant log aggregation system offered as an alternative to Elasticsearch as a log store for the {logging}. | ||
|
||
Elasticsearch indexes incoming log records completely during ingestion. Loki only indexes a few fixed labels during ingestion and defers more complex parsing until after the logs have been stored. This means Loki can collect logs more quickly. | ||
include::snippets/logging-loki-statement-snip.adoc[] |
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.
Please ignore this file (Not sure how it got added in this commit). I will remove this file in next commit.
|
||
Loki is a horizontally scalable, highly available, multi-tenant log aggregation system offered as a GA log store for {logging} {for} that can be visualized with the OpenShift {ObservabilityShortName} UI. The Loki configuration provided by OpenShift {logging-uc} is a short-term log store designed to enable users to perform fast troubleshooting with the collected logs. For that purpose, the {logging} {for} configuration of Loki has short-term storage, and is optimized for very recent queries. For long-term storage or queries over a long time period, users should look to log stores external to their cluster. | ||
|
||
Elasticsearch indexes incoming log records completely during ingestion. Loki indexes only a few fixed labels during ingestion and defers more complex parsing until after the logs have been stored. This means Loki can collect logs more quickly. No newline at end of file |
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.
Please ignore this file (Not sure how it got added in this commit). I will remove this file in next commit.
// | ||
:_mod-docs-content-type: SNIPPET | ||
|
||
Loki is a horizontally scalable, highly available, multi-tenant log aggregation system offered as a GA log store for {logging} {for} that can be visualized with the OpenShift {ObservabilityShortName} UI. The Loki configuration provided by OpenShift {logging-uc} is a short-term log store designed to enable users to perform fast troubleshooting with the collected logs. For that purpose, the {logging} {for} configuration of Loki has short-term storage, and is optimized for very recent queries. For long-term storage or queries over a long time period, users should look to log stores external to their cluster. No newline at end of file |
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.
Please ignore this file (Not sure how it got added in this commit). I will remove this file in next commit.
OBSDOCS-1166-Loki Statement of Purpose
I am planning to create a new PR and later close this PR as I am getting some problems with Git for this PR. |
@smunje1: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
OBSDOCS-1170 : log6x-clf.adoc - include verification / modifications
Aligned team: Observability - Logging
OCP version for cherry-picking:
JIRA issues: OBSDOCS-1170
Preview pages: https://78477--ocpdocs-pr.netlify.app/openshift-enterprise/latest/observability/logging/logging-6.0/log6x-clf.html
SME review requested: @jcantrill
QE review requested: @anpingli
Peer review requested: