Adiciona o logger v2 para criar arquivos de log de nome único e que não rotacionam internamente#13
Adiciona o logger v2 para criar arquivos de log de nome único e que não rotacionam internamente#13gabrielferreira-gaudium wants to merge 4 commits intodevfrom
Conversation
Summary of ChangesHello @gabrielferreira-gaudium, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! Este pull request introduz uma nova versão do sistema de logging, o Logger V2, que visa otimizar a forma como os arquivos de log são gerados e gerenciados. Ao invés de múltiplos arquivos rotacionados internamente, a V2 cria um arquivo único por tópico, simplificando a estrutura e a manutenção. A mudança é implementada de forma a permitir a escolha entre a versão original (V1) e a nova (V2), garantindo compatibilidade e flexibilidade para os serviços que utilizam a biblioteca. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
Este pull request introduz uma nova versão (v2) do logger, que simplifica a nomenclatura dos arquivos de log e remove a rotação interna, ao mesmo tempo que mantém a versão antiga (v1) para retrocompatibilidade. A arquitetura utiliza um padrão Facade para alternar entre as versões.
A refatoração é bem-vinda e a nova implementação v2 é mais simples e performática ao evitar a comunicação por canais e ao fazer cache dos loggers.
No entanto, identifiquei alguns pontos críticos que precisam de atenção:
- Segurança de Concorrência (Race Condition): A forma como a versão do logger é selecionada através de uma variável global (
currentVersion) não é segura para uso em goroutines concorrentes, o que pode levar a comportamento inesperado ou pânico. - Tratamento de Erros: A nova implementação v2 pode descartar logs silenciosamente se houver uma falha ao abrir o arquivo de log, o que é um comportamento arriscado para um sistema de logging.
- Permissões de Arquivo: A implementação v1 (que foi movida) utiliza permissões de arquivo e diretório inseguras, o que foi corrigido na v2. Seria bom aplicar a mesma correção na v1 para consistência e segurança.
Detalhei esses pontos nos comentários específicos no código. No geral, as mudanças são positivas, mas os problemas de concorrência e perda de logs precisam ser resolvidos antes do merge.
| var currentVersion = V1 | ||
|
|
||
| // fileStreamType é usado na comunicação por canal | ||
| type fileStreamType struct { | ||
| filename string | ||
| subject string | ||
| chReturn chan fileLogType | ||
| // SetVersion chooses which version to use (V1 or V2). | ||
| // This should ideally be called before InitializeMchLog. | ||
| func SetVersion(v LogVersion) { | ||
| currentVersion = v | ||
| } |
There was a problem hiding this comment.
A variável global currentVersion é acessada e modificada por múltiplas funções (SetVersion, LogSubject, InitializeMchLog, etc.) sem qualquer tipo de sincronização. Isso cria uma condição de corrida (race condition) em um ambiente com múltiplas goroutines. Por exemplo, se uma goroutine chama SetVersion enquanto outra chama LogSubject, o comportamento é indefinido e pode causar pânicos.
Para corrigir isso, o acesso à variável currentVersion deve ser protegido por um mutex (como sync.RWMutex).
Exemplo de correção:
import "sync"
var (
currentVersion = V1
versionMutex sync.RWMutex
)
func SetVersion(v LogVersion) {
versionMutex.Lock()
defer versionMutex.Unlock()
currentVersion = v
}
// Nas outras funções que leem currentVersion:
func (l *LogType) LogSubject(...) {
versionMutex.RLock()
version := currentVersion
versionMutex.RUnlock()
if version == V1 {
// ...
} else {
// ...
}
}Considerando que a versão deve ser definida idealmente antes da inicialização, uma alternativa mais segura seria remover a função SetVersion e passar a versão como parâmetro para InitializeMchLog, evitando o estado global mutável.
| f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664) | ||
| if err != nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
Se os.OpenFile falhar (por exemplo, por falta de permissão ou disco cheio), a função retorna silenciosamente (return). Isso faz com que a mensagem de log seja perdida sem qualquer indicação do problema. Isso é perigoso, pois a aplicação pode estar falhando em registrar logs importantes sem que ninguém perceba.
A implementação V1 possuía um fallback para logar o erro no console. A V2 deveria ter um comportamento semelhante, logando o erro de abertura do arquivo no stderr para garantir que a falha seja visível. Para isso, será necessário importar o pacote github.com/rs/zerolog/log.
f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664)
if err != nil {
log.Error().Err(err).Str("filename", filename).Msg("mchlogcoreV2: falha ao abrir arquivo de log")
return
}
MchLogToolkitGo - Logger v2
Descrição
Adiciona ao MchLogToolkitGo uma versão atualizada do logger (chamada v2) na qual os arquivos gerados de um tipo (parâmetro subject) são chamados
/<subject>/<subject>.log. Por ex., para o tipo "error", o arquivo será:/error/error.log.Com essa alteração, não mais serão gerados múltiplos arquivos de log de cada tipo, e a rotação de logs não será mais feita internamente pelo MchLogToolkitGo.
É possível determinar a versão do logger a ser utilizada a partir da função pública
SetVersion().A estrutura dos arquivos foi levemente modificada:
mchlogcoreV1/mchlogV1.go: Exatamente o mesmo código do logger atual (o que reside emmchlogcore/mchlog.go)mchlogcoreV2/mchlogV2.go: Versão atualizada do loggermchlogcore/mchlog.go: Agora serve para configurar qual a versão do logger deve ser utilizadaTipo de Mudança
Checklist
Como testar as alterações
Ao testar um um serviço que use o novo logger, ao inicializar o serviço, a primeira mensagem em
/applog/<serviço>/info/info<?>.logdeve ser a versão do logger utilizada.Pelo nome dos arquivos de log gerados nas pastas dentro de
/applog/<serviço>/do serviço, é possível determinar a versão do logger também.Impactos no Deploy
Importante adicionar aos microsserviços que usarem este logger com a implementação atual uma flag no arquivo .conf para determinar a versão (v1 ou v2); sendo a padrão a v1.
Dependências
Não há.
Issues
Não há.
Observações Adicionais
Não há.