feat(observability): expandir AdicionarObservabilidade + OTLP sink Serilog + correlation_id span tag#367
Conversation
…rilog + correlation_id span tag Expansão substantiva da extension method existente em Infrastructure.Core. Sem mudança de comportamento ainda — extension não é invocada por nenhum Program.cs neste PR (wiring é o PR 3 da entrega da Story #30). OpenTelemetryConfiguration.AdicionarObservabilidade - Aceita IConfiguration + IHostEnvironment + nomeServico - Toggle Observability:Enabled (default true; off-switch para CI sem Collector e troubleshooting com stack LGTM fora do ar) - Tracing: AspNetCore + EF Core + HttpClient + ActivitySource(nomeServico) + OTLP - Metrics: AspNetCore + Runtime + HttpClient + Meter(nomeServico) + Wolverine + OTLP - Sampler: AlwaysOn em Development, ParentBased(TraceIdRatio 10%) demais (ADR-0018) - Resource: service.name + service.namespace=uniplus + deployment.environment SerilogConfiguration.ConfigurarSerilog - Sobrecarga adicional aceita nomeServico para popular ResourceAttributes do sink OTLP (service.name + service.namespace) — necessário porque o pipeline Serilog→OTLP é independente do pipeline OTel SDK e ambos precisam rotular logs/traces com a mesma identidade no Loki/Tempo - Sobrecarga sem nomeServico é mantida para callers ainda não migrados; PR 3 atualiza os 3 Program.cs para usar a sobrecarga rotulada - Sink WriteTo.OpenTelemetry com OtlpProtocol.Grpc + IncludedData (TraceIdField | SpanIdField | MessageTemplateTextAttribute) — fecha Story #105 colateralmente - Console sink preservado (docker logs continua útil em bootstrap debugging) - PiiMaskingEnricher mantém posição pré-sinks (ordem natural Serilog) — CPF mascarado antes de qualquer egress (ADR-0011) CorrelationIdMiddleware - Activity.Current?.SetTag("correlation_id", correlationId) após writer.SetCorrelationId, antes de LogContext.PushProperty — habilita drill-down Loki→Tempo via derivedFields (ADR-0018). Fecha Story #110 colateralmente - Null-safe via ?. — middleware funciona sem OTel wired (testes unitários sem ActivityListener registrado) Testes - Novo OpenTelemetryConfigurationTests (7 testes): toggle on/off/ausente, guards de argumento (services/config/env null + nome vazio) - CorrelationIdMiddlewareTests +2 testes: com Activity ativa seta tag, sem Activity não falha Build: 0 warning, 0 error. Suite unit + arch tests: 534 passed (incluindo 9 novos), 0 failed. Refs #30, #105, #110
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fed36987a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jf2s
left a comment
There was a problem hiding this comment.
Revisão: PR #367 — feat(observability): expandir AdicionarObservabilidade + OTLP sink Serilog + correlation_id span tag
Tipo de PR: substancial (3 arquivos de produção tocados + 1 novo arquivo de teste + 1 estendido). Sem mudança de comportamento em runtime ainda — wiring vem no PR 3.
Diff: 387 insertions, 5 deletions em 5 arquivos.
Resumo
PR 2 da entrega da Story #30, alinhado com o plano de 3 PRs incrementais. Expande AdicionarObservabilidade substantivamente (de 22 → 107 linhas) cobrindo todas as instrumentações CAs da issue. Adiciona sink OTLP no Serilog (fecha #105 colateralmente) e span tag correlation_id no CorrelationIdMiddleware (fecha #110 colateralmente). 9 novos testes unit. ADRs 0011 e 0018 respeitadas. CI 5/5 verde, build/test em 3m31s.
Findings
Bloqueantes (0)
Nenhum. Mudanças isoladas em Infrastructure.Core, sem impacto em runtime existente até o wire do PR 3.
Importantes (1)
[I1] CA "Testes unitários para seleção de sampler por ambiente" da #30 não está coberto pelos novos testes
A issue #30 lista explicitamente entre seus CAs: "Testes unitários para registro DI e seleção de sampler por ambiente". Os 7 testes em OpenTelemetryConfigurationTests.cs cobrem registro DI (toggle on/off/default + guards), mas a seleção condicional do sampler (AlwaysOnSampler em Development vs ParentBasedSampler(TraceIdRatioBasedSampler(0.1)) nos demais) não tem cobertura.
A justificativa implícita do PR é que TracerProvider não expõe Sampler publicamente — verdade, mas resolvível com:
- (a) Reflection-based assertion no
Sdk.GetType().GetField(\"_sampler\", BindingFlags.NonPublic | BindingFlags.Instance)— frágil mas explícita - (b) Test indireto via
InMemoryExporterrodando 1 span e contando o que foi exportado em ratio (estatístico — flaky, evitar) - (c) Refatorar
OpenTelemetryConfigurationpara extrair a seleção do sampler como método interno testável:internal static Sampler SelecionarSampler(IHostEnvironment environment)com[InternalsVisibleTo]já existente no.csprojpara*.UnitTests— limpo, sem reflection, valida a regra explicitamente
Recomendo (c) — alinha com o pattern já estabelecido em Errors//Cryptography/ que extraem helpers internos para testabilidade. Estimativa: 15min (extrair método + 2 testes).
Sugestões (2)
[S1] CA "OTLP exporter usa endpoint da env quando presente" deferido para integration test do PR 3
A asserção sobre o endpoint OTLP funcionar via env var OTEL_EXPORTER_OTLP_ENDPOINT é difícil de testar em unit (envolve I/O do exporter). Aceitável deferir para o integration test com Testcontainers do OTEL Collector que vem no PR 3 — só vale documentar essa decisão explicitamente no body da issue ou no PR 3 quando for criado, evitando que o CA pareça "esquecido" na auditoria.
[S2] Sobrecarga sem nomeServico em ConfigurarSerilog cria 2 caminhos temporariamente
Os 3 Program.cs em main ainda usam ConfigurarSerilog(context.Configuration) (sem nomeServico). PR 3 troca todos para a sobrecarga rotulada. Janela de risco: 1 PR. Não é bloqueante, mas vale lembrar:
- A sobrecarga sem nome registra o sink OTLP sem
service.name— logs vão para Loki sem rotulação até o PR 3 mergar - Se PR 3 demorar significativamente, considerar
[Obsolete(\"Use a sobrecarga com nomeServico — esta variante deixa de rotular logs no Loki\")]na sobrecarga sem nome para sinalizar a transição
Pontos positivos
- Comentários técnicos com WHY ancorado em ADR — cada decisão não-óbvia (sampler condicional, namespace canônico, posição do PiiMaskingEnricher, drill-down Loki↔Tempo) cita ADR específico ou explica a invariante. Exemplos: doc do
EnabledConfigurationKeycita o cenário de troubleshooting,ProductionSamplingRatioesclarece divisão API/Collector na sampling adaptativa - Constantes nomeadas em vez de magic strings —
EnabledConfigurationKey,ServiceNamespaceResourceValue,ProductionSamplingRatio,ActivityTagNamereduzem chance de drift entre código e testes - Sobrecarga preserva backward compat — escolha pragmática: o método antigo continua funcional, o novo agrega capacidade. Evita quebrar 3
Program.csno escopo do PR 2 - Null-safety explícita no middleware + teste dedicado (
SemActivityAtiva_NaoDeveFalhar) que documenta o invariante - Workflow correto — PR 1 (#365) já mergeado, branch criada de main fresca, sem conflitos esperados
- PR description completa — tabela de mudanças por arquivo, validação local explícita, ADRs respeitadas listadas, decisões deferidas com justificativa
Veredicto
MUDANÇAS NECESSÁRIAS (1 finding importante)
O finding [I1] aponta CA explícito da issue #30 não coberto. A correção via opção (c) — extrair SelecionarSampler como internal static com InternalsVisibleTo para testes — é mecânica e curta (~15min). Sugestões [S1]/[S2] não bloqueiam mas vale tratar S1 com nota explícita no PR 3 quando for criado.
…cionarSampler Aplica feedback de review do PR #367. Wolverine ActivitySource em WithTracing - Tracing pipeline registrava apenas .AddSource(nomeServico), deixando spans Wolverine (commands, handlers, outbox) fora do Tempo. Adiciona .AddSource(WolverineActivityAndMeterName) para fechar a correlação end-to-end com HTTP/EF spans (ADR-0018). - Constante WolverineActivityAndMeterName extraída como public const para reuso consistente entre WithTracing.AddSource e WithMetrics.AddMeter e para que testes referenciem sem string literal. SelecionarSampler como internal static testável - Extraído para tornar a regra de seleção (AlwaysOn em Development vs ParentBased(TraceIdRatio 10%) demais — ADR-0018) cobrível por unit test sem reflection no campo _sampler privado do TracerProvider. - InternalsVisibleTo já existente em Infrastructure.Core.csproj expõe para Unifesspa.UniPlus.Infrastructure.Core.UnitTests. - 6 novos testes: - AlwaysOn em Development - ParentBased+TraceIdRatio 10% em Production/Staging/HML/Test (Theory) - ArgumentNullException em environment null Build: 0 warning, 0 error. Suite unit + arch tests: 540 passed (incluindo 6 novos), 0 failed. Refs #30, #105, #110
jf2s
left a comment
There was a problem hiding this comment.
Aprovado.
Findings de review endereçados no commit 5072c39:
- [Codex P2]
AddSource(WolverineActivityAndMeterName)adicionado emWithTracing— spans Wolverine de commands/handlers/outbox agora vão para o Tempo correlacionados com HTTP/EF spans (ADR-0018). Thread inline resolvido. - [I1]
SelecionarSamplerextraído comointernal static(visível ao test project viaInternalsVisibleTojá existente). 6 novos testes cobrem AlwaysOn em Development, ParentBased+TraceIdRatio 10% em Production/Staging/HML/Test (Theory), e guard ArgumentNullException — sem reflection no_samplerprivado do TracerProvider. - [S1] Endpoint OTLP via env var será coberto pelo integration test Testcontainers do PR 3 (vou registrar essa nota explícita no body do PR 3 ao criar).
- [S2] Sobrecarga sem
nomeServicoemConfigurarSerilogaceita como débito de 1 PR — PR 3 troca os 3Program.cspara a sobrecarga rotulada.
CI 5/5 verde após fix. Suite local: 540 testes passando, 0 warning, 0 error. Pode mergear.
…n test ponta-a-ponta PR 3 (final) da entrega da Story #30. Wire concreto da extension method expandida no PR #367 nos 3 APIs do projeto + integration test com Testcontainers do OTEL Collector validando o pipeline OTLP gRPC end-to-end. Wire em Program.cs (Selecao + Ingresso + Portal) - builder.Services.AdicionarObservabilidade(nomeServico, config, env) registrado após AddRequestLogging em cada API - nomeServico declarado como const local (uniplus-{modulo}) para garantir igualdade estrita entre o Resource OTel SDK e o ResourceAttributes do sink OTLP do Serilog — drift de naming entre logs/traces seria a primeira coisa a quebrar drill-down Loki↔Tempo no Grafana - builder.Host.UseSerilog estendido para passar nomeServico à sobrecarga rotulada de ConfigurarSerilog (PR #367) 3 appsettings.json - Section Observability:Enabled=true (default seguro pra produção) - OTEL_EXPORTER_OTLP_ENDPOINT é env var standard do OTel SDK (NÃO IConfiguration); configurado via env vars do container/runtime, não em appsettings ApiFactoryBase - AddInMemoryCollection com Observability:Enabled=false ANTES dos GetConfigurationOverrides — suites HTTP-only sem Collector OTel provisionado silenciam o exporter, evitando ruído de drop batches/retry no CI - Override segue padrão de InfraHealthCheckNamesToRemoveForTests OtelCollectorContainerFixture (Tests.Fixtures/Hosting) - Sobe otel/opentelemetry-collector-contrib:0.117.0 com config minimal (receivers OTLP gRPC+HTTP, processor batch 100ms, exporter debug verbosity=detailed) - Config YAML inline via WithResourceMapping (sem arquivo temp em disco) - Collection compartilhada "OtelCollector" — pattern análogo a MinIO/Vault/Keycloak - GetLogsAsync() concatena stdout+stderr (Collector loga exporter debug em stderr) OpenTelemetryWiringTests - 1 teste end-to-end: AdicionarObservabilidade → ActivitySource → OtlpExporter gRPC → Collector real → exporter debug → asserts em ResourceSpans + service.name + service.namespace + nome do span emitido - Set/restore de OTEL_EXPORTER_OTLP_ENDPOINT via try/finally — xUnit não paraleliza dentro da mesma collection, sem race com outros testes - TestHostEnvironment stub local (sem NSubstitute — IntegrationTests project mantém deps lean: só Testcontainers + AwesomeAssertions) Build: 0 warning, 0 error. Suite COMPLETA: 629 testes passando, 0 falhas. Refs #30 Refs #105 Refs #110
Resumo
PR 2 da entrega da Story #30 (
wiring OpenTelemetry). Expande a extension methodAdicionarObservabilidade(atualmente 22 linhas, só com AspNetCore tracing) para o conjunto canônico Uni+ exigido pelos CAs da issue + ADR-0018; estendeConfigurarSerilogcom sink OTLP fechando colateralmente a Story #105; adiciona span tagcorrelation_idaoCorrelationIdMiddlewarefechando colateralmente a Story #110.Sem mudança de comportamento ainda — extension não é invocada por nenhum
Program.csneste PR. Wiring nos 3 APIs + integration test Testcontainers segue no PR 3 da entrega, que é quem efetivamente vai fechar #30, #105 e #110.Mudanças
OpenTelemetryConfiguration.cs(Infrastructure.Core/Observability)IConfiguration+IHostEnvironment+nomeServicoObservability:Enabled(defaulttrue; off-switch para CI sem Collector e troubleshooting com stack LGTM fora do ar)nomeServico) + OTLP gRPCnomeServico) + Wolverine (instrumentação built-in via Meter nativo) + OTLP gRPCAlwaysOnSampleremDevelopment,ParentBasedSampler(TraceIdRatioBasedSampler(0.1))nos demais — ADR-0018 head-based 10%; tail-based 100% para erro/latência fica no Collectorservice.name,service.namespace=uniplus,deployment.environment=<env>— particiona dashboards GrafanaSerilogConfiguration.cs(Infrastructure.Core/Logging)ConfigurarSerilog(this LoggerConfiguration, IConfiguration, string? nomeServico)— populaResourceAttributesdo sink OTLP comservice.name+service.namespace(necessário porque pipeline Serilog→OTLP é independente do pipeline OTel SDK; ambos precisam rotular o mesmo serviço para correlação log↔trace no Loki/Tempo)nomeServicomantida para callers ainda não migrados — PR 3 atualiza os 3Program.cspara usar a sobrecarga rotuladaWriteTo.OpenTelemetrycomOtlpProtocol.Grpc+IncludedData = TraceIdField | SpanIdField | MessageTemplateTextAttribute— fecha Story story(observability): Serilog sink para Loki + service.name enrichment por ambiente #105 colateralmentedocker logscontinua útil em bootstrap debuggingPiiMaskingEnrichermantém posição pré-sinks (ordem natural Serilog) — CPF mascarado antes de qualquer egress (Console ou OTLP) → ADR-0011CorrelationIdMiddleware.cs(Infrastructure.Core/Middleware)Activity.Current?.SetTag(\"correlation_id\", correlationId)apóswriter.SetCorrelationIde antes deLogContext.PushProperty— habilita drill-down Loki→Tempo viaderivedFieldsno Grafana (ADR-0018), fechando o ciclo log↔trace↔dashboard. Fecha Story story(observability): propagar correlation_id como Activity span attribute #110 colateralmente?.— middleware funciona sem OTel wired (testes unitários semActivityListenerregistrado)ActivityTagName = \"correlation_id\"para os testes referenciarem sem string literalTestes adicionados
OpenTelemetryConfigurationTests.cs(novo)7 testes cobrindo:
TracerProvidernemMeterProvidertrue(seguro pra produção)services,configuration,environmentnull +nomeServicovazio)CorrelationIdMiddlewareTests.cs(+2 testes)InvokeAsync_ComActivityAtiva_DeveSetarCorrelationIdComoTagDoSpan— registraActivityListenertemporário comSample = AllData, validaActivity.GetTagItem(ActivityTagName)InvokeAsync_SemActivityAtiva_NaoDeveFalhar— semActivityListener,Activity.Currentpermanece null durante o pipeline; valida que?.mantém o middleware safeValidação local
```bash
dotnet build UniPlus.slnx # 0 warning, 0 error
dotnet test UniPlus.slnx --filter "FullyQualifiedName!~IntegrationTests" # 534 passed
dotnet format UniPlus.slnx --verify-no-changes # exit 0
```
ADRs respeitadas
PiiMaskingEnrichercontinua antes dos sinks (ordem natural). CPF mascarado antes de qualquer egressDecisões fora do escopo (deferidas)
OpenTelemetry.Instrumentation.Npgsql— EF Core instrumentation já cobre spans Postgres por baixo viaDbCommandeventstail_sampling_processorno Collector, não da API/health/observabilityvalidando exporter — não no escopo da Story story(observability): wiring de OpenTelemetry nos Program.cs #30; vira issue separada se necessárioPróximo PR (PR 3, fecha #30 + #105 + #110)
AdicionarObservabilidadenos 3Program.cs(Selecao + Ingresso + Portal — issue só fala em Selecao+Ingresso, mas Portal nasceu depois e merece pela consistência)ConfigurarSerilog(comnomeServico)appsettings.json+ 3appsettings.Development.jsoncomObservability:EnabledRefs #30, #105, #110