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

Logs no longer include call stack #53

Closed
jlewi opened this issue Apr 16, 2024 · 3 comments · Fixed by #54
Closed

Logs no longer include call stack #53

jlewi opened this issue Apr 16, 2024 · 3 comments · Fixed by #54

Comments

@jlewi
Copy link
Owner

jlewi commented Apr 16, 2024

#51 changed how we setup the loggers in order to write to two different sinks.
As a side effect I think we stopped logging the caller.
@jlewi
Copy link
Owner Author

jlewi commented Apr 16, 2024

Previously we were doing

c := zap.NewDevelopmentConfig()
log, err := c.Build()
if err != nil {
    t.Fatalf("Failed to create logger: %v", err)
}

@jlewi
Copy link
Owner Author

jlewi commented Apr 16, 2024

It looks like that calls cfg.buildOptions on the writeSyncer
https://github.com/uber-go/zap/blob/3ffa0c00594205c2c623aa9493847217982b558d/config.go#L264

Here we can add the option to AddCaller
https://github.com/uber-go/zap/blob/3ffa0c00594205c2c623aa9493847217982b558d/config.go#L272

@jlewi
Copy link
Owner Author

jlewi commented Apr 16, 2024

This turns out to be a one line fix

newLogger = newLogger.WithOptions(zap.AddCaller())
zap.ReplaceGlobals(newLogger)

Fix is in #54

@jlewi jlewi mentioned this issue Apr 17, 2024
Merged
@jlewi jlewi closed this as completed in #54 Apr 17, 2024
jlewi added a commit that referenced this issue Apr 17, 2024
@jlewi
Jlewi/analyze (#54)
da26d1b 
Logging to support #7 

* Updated logging in Agent.Generate and Executor.Execute so we log the
information needed to determine when the human edits the cell

* To properly log protos in structured logging we need to use the zapr
proto plugin to generate MarshalLogObject functions for all the protos.
We can then use `zap.Object` to pass the protos as fields to logger
  * We need to enable logr to allow passing zap fields

* Add a logs subcommand and analyzer to process the logs and join the
log entries into traces and logs for the blocks

* Log call stack; fix #53; this was broken when we stopped using the
helper functions to build the logs. Now that we explicitly build the
logger we need to add the option to log the call stack.

* Altered the `SetupLogging` method in `app/cmd/assets.go` to accept a
`logToFile` boolean parameter, allowing conditional logging to files. We
only want the serve command to log to files. We don't need the other
commands to log to files


#### Analysis Package (`app/pkg/analyze`):
- Developed a new package, `analyze`, responsible for processing and
analyzing log files. This includes parsing raw log entries, organizing
them into traces and blocks, and producing structured summaries that
highlight the execution flow and outcomes.
- Implemented several utility types and methods (`LogEntry`, `Trace`,
`BlockLog`, etc.) to facilitate the analysis and organization of logs.

#### Configuration and Utility Changes:
- Extended the `app/pkg/config/config.go` to include methods for
retrieving both raw and processed log directories, supporting the new
logging mechanism.
- Added utility functions in `app/pkg/docs/outputs.go` to extract
specific metadata from logged blocks, such as execution exit codes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Logging to support catching errors jlewi/foyle
1 participant
@jlewi