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

Improve logging #41

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Improve logging #41

merged 1 commit into from
Jul 18, 2023

Conversation

Kidswiss
Copy link
Contributor

@Kidswiss Kidswiss commented Jul 13, 2023

Summary

  • Remove all object prints from the debug logs.
  • Add metatada to the logger before running the transformation
  • Ability to print whole functionIO to the events

If the GRPC server is in devmode, it will save the funcIOs of each function to configMaps in the default namespace. It will always keep two configMaps per composite and function. One contains the previous and one the current state. For example for VSHNPostgreSQL this could result in up to four configMaps; two for the postgresql function and two for the miniodev function (which provides the local S3 bucket).

These configMaps can be used to see the state of the whole FuncIO and should help with local debugging. Additionally the GRPC server now prints a diff of those configMaps on each reconcile.

The configMaps and the diffs will only rotate on .spec changes on the claim/composite, it uses the generation to track current and previous states. This way we can track the "logical" diff of a change, even if it needs multiple reconcile loops to be applied fully.

Checklist

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • Update tests.
  • Link this PR to related issues.

@Kidswiss Kidswiss marked this pull request as draft July 13, 2023 13:57
@zugao
Copy link
Collaborator

zugao commented Jul 14, 2023

As already discussed, for prepareFuncLogger() is a great idea but we should only get the composite name as the other 2 somehow are redundant. Meanwhile appendFuncIOToMessages() does not add in my opinion much value, with this approach we move out the function io from logs to events. It helps cleaning out the logs but it will be more difficult to connect the logs to the function io later on. Anyway for debugging purposes it should be fine though we need to find a better approach like having only the diff?

@Kidswiss
Copy link
Contributor Author

I'm looking into the diff idea.

@Kidswiss Kidswiss force-pushed the change/logging branch 3 times, most recently from 0572183 to 1d28c06 Compare July 17, 2023 12:43
@Kidswiss Kidswiss added the enhancement New feature or request label Jul 17, 2023
@Kidswiss Kidswiss marked this pull request as ready for review July 17, 2023 12:44
Copy link
Collaborator

@zugao zugao left a comment

Choose a reason for hiding this comment

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

Seems good so far haven't tested it though.

Copy link
Contributor

@glrf glrf left a comment

Choose a reason for hiding this comment

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

LGTM overall

pkg/comp-functions/functions/helper/funcio.go Show resolved Hide resolved
pkg/comp-functions/functions/helper/funcio.go Outdated Show resolved Hide resolved
pkg/comp-functions/functions/helper/funcio.go Outdated Show resolved Hide resolved
* Remove all object prints from the debug logs.
* Add metatada to the logger before running the transformation
* Ability to print whole functionIO to the events

If the GRPC server is in devmode, it will save the funcIOs of each
function to configMaps in the `default` namespace. It will always keep
two configMaps per composite and function. One contains the previous and
one the current state. For example for VSHNPostgreSQL this could result
in up to four configMaps; two for the postgresql function and two for
the miniodev function (which provides the local S3 bucket).

These configMaps can be used to see the state of the whole FuncIO and
should help with local debugging. Additionally the GRPC server now
prints a diff of those configMaps on each reconcile.

The configMaps and the diffs will only rotate on `.spec` changes on the
claim/composite, it uses the generation to track current and previous
states. This way we can track the "logical" diff of a change, even if
it needs multiple reconcile loops to be applied fully.
@Kidswiss Kidswiss merged commit af99cb9 into master Jul 18, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants