Skip to content

Conversation

jviau
Copy link
Contributor

@jviau jviau commented Oct 1, 2025

Issue describing the changes in this PR

resolves #11400

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

  • Backporting to the in-proc branch is not required
    • Otherwise: Link to backporting PR
  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md -- TODO
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • I have added all required tests (Unit tests, E2E tests)

Additional information

This PR introduces a mechanism where WebHost services can opt into having their logs forwarded to the active ScriptHost ILogger when it is available. When a ScriptHost is not available, logs will go to the WebHost ILogger. The implementation automatically tracks changes to active ScriptHost, while avoiding memory leaks through the use of WeakReference<T> (allowed active ScriptHost ILogger to be cleaned up on ScriptHost shutdown).

Usage

Simply add [ForwardingLogger] attribute to ILogger<T> or ILoggerFactory import:

public class MyService([ForwardingLogger] ILogger<MyService> logger)
{
}

TODO

  • Evaluate how/if the added keyed services are being proxied to the ScriptHost services I have excluded ILoggerFactory and ILogger<> from being copied to script host service container. They were not actually used as ScriptHost overrode with their own, but the copying process did fail validation.

@jviau jviau requested a review from a team as a code owner October 1, 2025 20:54
@Copilot Copilot AI review requested due to automatic review settings October 1, 2025 20:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a mechanism for WebHost services to forward their logs to the active ScriptHost logger when available, falling back to the WebHost logger when no ScriptHost is active. The implementation uses WeakReference to avoid memory leaks and automatically tracks changes to the active ScriptHost.

Key changes:

  • Introduces ForwardingLogger classes that automatically switch between WebHost and ScriptHost loggers
  • Adds a ForwardingLoggerAttribute for dependency injection that uses keyed services
  • Updates IScriptHostManager interface to expose Services property for accessing ScriptHost services

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/WebJobs.Script/Host/IScriptHostManager.cs Adds Services property and enables nullable reference types
src/WebJobs.Script/Diagnostics/ForwardingLogger.cs Core forwarding logger implementation with weak references
src/WebJobs.Script/Diagnostics/ForwardingLoggerFactory.cs Factory for creating forwarding loggers
src/WebJobs.Script/Diagnostics/ForwardingLoggerAttribute.cs Attribute for keyed service injection
src/WebJobs.Script/Extensions/ScriptLoggingBuilderExtensions.cs Extension methods for registering forwarding logger services
src/WebJobs.Script/Diagnostics/HealthChecks/TelemetryHealthCheckPublisher.cs Example usage of ForwardingLogger attribute
src/WebJobs.Script.WebHost/Program.cs Registers forwarding logger in web host startup
src/WebJobs.Script.WebHost/Diagnostics/ILoggingBuilderExtensions.cs Formatting and return type improvements
test/ files Comprehensive unit tests for all new functionality

Add mechanism to forward logs from WebHost to ScriptHost
@jviau jviau force-pushed the u/jviau/log-forwarder branch from 910a686 to 46b1a61 Compare October 1, 2025 20:56
@Azure Azure deleted a comment from Copilot AI Oct 1, 2025
/// <summary>
/// Gets the current <see cref="IServiceProvider"/> for the active Script Host.
/// </summary>
IServiceProvider? Services { get; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have found a couple times recently that having access to IServiceProvider here is useful. Not sure if there is a reason we didn't expose it in the past?

@jviau jviau force-pushed the u/jviau/log-forwarder branch from 69cd8f7 to 9d5193b Compare October 1, 2025 21:25
@jviau
Copy link
Contributor Author

jviau commented Oct 1, 2025

serviceProvider.CreateChildContainer(..) does not currently support keyed services. We will need to work on that. Or in this case, I can just block ILoggerFactory from propagating. It isn't used anyways - the ScriptHost overrides it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HealthChecks] TelemetryHealthCheckPublisher does not emit logs to ScriptHost/customer logs
1 participant