use default bool for enable-var-log-collection

[rootfs]

enable-var-log-collection defaults to false. Once turned on, logging sidecars are created, causing pod startup latency increase.

This change reduces startup latency from ~14 seconds to ~3 seconds on a cold start.

thanks @markusthoemmes for his inspiration in investigating latency.

Restore default and add more comments

Fixes #

Proposed Changes

Release Note

Comment out logging.enable-var-log-collection config for controller.

[mdemirhan]

Might be better to do:

-  logging.enable-var-log-collection: "false"

And not rely on what implementation has by default.

[rootfs]

+1, updated

[mdemirhan]

/lgtm
/approve

[rootfs]

/assign @evankanderson

[evankanderson]

/approve

This feels like it would be better handled by a PodPreset.

Is this a temporary, or permanent solution? The lack of easy access to logs is a [major observability problem for serverless architectures](too lazy to cite), so centralized logging seems like a useful way to address this.

One other way which we could address this which would avoid creating so many pods would be to run the log collector as a DaemonSet, and then using a PodPreset to map volumes from the DaemonSet into each targeted Pod.

Much of this work may also be applicable to upstream k8s work (observation).

/end shouting into the void.

[google-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, mdemirhan, rootfs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [evankanderson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[evankanderson]

This feels like it should have broken an integration test -- we should have some tests for the logs aggregation that verify that logs from controlled and monitored pods end up in a storage system.

[mdemirhan]

@evankanderson Agreed on both daemonset as well as integration tests. Both are captured as Github issues, sadly waiting for owners :)