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

Use ILogger<T> for webjob extensions #22362

Merged
merged 1 commit into from
Jul 1, 2021
Merged

Use ILogger<T> for webjob extensions #22362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...grid/Microsoft.Azure.WebJobs.Extensions.EventGrid/src/EventGridExtensionConfigProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void Initialize(ExtensionConfigContext context)
throw new ArgumentNullException(nameof(context));
}

_logger = _loggerFactory.CreateLogger(LogCategories.CreateTriggerCategory("EventGrid"));
_logger = _loggerFactory.CreateLogger<EventGridExtensionConfigProvider>();

#pragma warning disable 618
Uri url = context.GetWebhookHandler();
Expand Down
9 changes: 8 additions & 1 deletion sdk/eventgrid/Microsoft.Azure.WebJobs.Extensions.EventGrid/tests/JobhostEndToEnd.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public async Task ConsumeEventGridEventTest_Single(string functionName)

await host.GetJobHost().CallAsync(functionName, args);
Assert.AreEqual(_functionOut, expectOut);

var categories = host.GetTestLoggerProvider().GetAllLogMessages().Select(p => p.Category);
CollectionAssert.Contains(categories, "Microsoft.Azure.WebJobs.Extensions.EventGrid.EventGridExtensionConfigProvider");
_functionOut = null;
}

Expand Down Expand Up @@ -354,7 +357,8 @@ public async Task OutputCloudEventBindingParamsTests(string functionName, string
});

ILoggerFactory loggerFactory = new LoggerFactory();
loggerFactory.AddProvider(new TestLoggerProvider());
var provider = new TestLoggerProvider();
loggerFactory.AddProvider(provider);
// use moq eventgridclient for test extension
var customExtension = new EventGridExtensionConfigProvider(eventConverter, new HttpRequestProcessor(NullLoggerFactory.Instance.CreateLogger<HttpRequestProcessor>()), loggerFactory);

Expand All @@ -368,6 +372,9 @@ public async Task OutputCloudEventBindingParamsTests(string functionName, string

await host.GetJobHost().CallAsync($"OutputCloudEventBindingParams.{functionName}");

var categories = provider.GetAllLogMessages().Select(p => p.Category);
CollectionAssert.Contains(categories, "Microsoft.Azure.WebJobs.Extensions.EventGrid.EventGridExtensionConfigProvider");

var expectedEvents = new HashSet<string>(expectedCollection.Split(' '));
foreach (CloudEvent eve in cloudEvents)
{
Expand Down
2 changes: 1 addition & 1 deletion ...icrosoft.Azure.WebJobs.Extensions.EventHubs/src/Config/EventHubExtensionConfigProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ internal static void LogExceptionReceivedEvent(ExceptionReceivedEventArgs e, ILo
private IAsyncCollector<EventData> BuildFromAttribute(EventHubAttribute attribute)
{
EventHubProducerClient client = _clientFactory.GetEventHubProducerClient(attribute.EventHubName, attribute.Connection);
return new EventHubAsyncCollector(new EventHubProducerClientImpl(client, _loggerFactory));
return new EventHubAsyncCollector(new EventHubProducerClientImpl(client, _loggerFactory.CreateLogger<EventHubProducerClientImpl>()));
}

private static string ConvertEventDataToString(EventData x)
Expand Down
6 changes: 3 additions & 3 deletions sdk/eventhub/Microsoft.Azure.WebJobs.Extensions.EventHubs/src/EventHubProducerClientImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
using Microsoft.Azure.WebJobs.Logging;
using Microsoft.Extensions.Logging;

namespace Microsoft.Azure.WebJobs
namespace Microsoft.Azure.WebJobs.EventHubs
{
/// TODO: Remove when https://github.com/Azure/azure-sdk-for-net/issues/9117 is fixed
internal class EventHubProducerClientImpl : IEventHubProducerClient
{
private readonly EventHubProducerClient _client;
private readonly ILogger _logger;

public EventHubProducerClientImpl(EventHubProducerClient client, ILoggerFactory loggerFactory)
public EventHubProducerClientImpl(EventHubProducerClient client, ILogger logger)
{
_client = client;
_logger = loggerFactory?.CreateLogger(LogCategories.Executor);
_logger = logger;
}

public async Task<IEventDataBatch> CreateBatchAsync(CancellationToken cancellationToken)
Expand Down
10 changes: 5 additions & 5 deletions sdk/eventhub/Microsoft.Azure.WebJobs.Extensions.EventHubs/src/Listeners/EventHubListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ internal sealed class EventHubListener : IListener, IEventProcessorFactory, ISca
private readonly bool _singleDispatch;
private readonly BlobsCheckpointStore _checkpointStore;
private readonly EventHubOptions _options;
private readonly ILogger _logger;

private Lazy<EventHubsScaleMonitor> _scaleMonitor;
private readonly ILoggerFactory _loggerFactory;

public EventHubListener(
string functionId,
Expand All @@ -40,9 +40,9 @@ public EventHubListener(
IEventHubConsumerClient consumerClient,
BlobsCheckpointStore checkpointStore,
EventHubOptions options,
ILogger logger)
ILoggerFactory loggerFactory)
{
_logger = logger;
_loggerFactory = loggerFactory;
_executor = executor;
_eventProcessorHost = eventProcessorHost;
_singleDispatch = singleDispatch;
Expand All @@ -54,7 +54,7 @@ public EventHubListener(
functionId,
consumerClient,
checkpointStore,
_logger));
_loggerFactory.CreateLogger<EventHubsScaleMonitor>()));
}

/// <summary>
Expand Down Expand Up @@ -82,7 +82,7 @@ public async Task StopAsync(CancellationToken cancellationToken)

IEventProcessor IEventProcessorFactory.CreateEventProcessor()
{
return new EventProcessor(_options, _executor, _logger, _singleDispatch);
return new EventProcessor(_options, _executor, _loggerFactory.CreateLogger<EventProcessor>(), _singleDispatch);
}

public IScaleMonitor GetMonitor()
Expand Down
8 changes: 4 additions & 4 deletions ...zure.WebJobs.Extensions.EventHubs/src/Triggers/EventHubTriggerAttributeBindingProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.Azure.WebJobs.EventHubs
{
internal class EventHubTriggerAttributeBindingProvider : ITriggerBindingProvider
{
private readonly ILogger _logger;
private readonly ILoggerFactory _loggerFactory;
private readonly IOptions<EventHubOptions> _options;
private readonly EventHubClientFactory _clientFactory;
private readonly IConverterManager _converterManager;
Expand All @@ -32,7 +32,7 @@ public EventHubTriggerAttributeBindingProvider(
_converterManager = converterManager;
_options = options;
_clientFactory = clientFactory;
_logger = loggerFactory?.CreateLogger(LogCategories.CreateTriggerCategory("EventHub"));
_loggerFactory = loggerFactory;
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
Expand All @@ -59,7 +59,7 @@ public Task<ITriggerBinding> TryCreateAsync(TriggerBindingProviderContext contex
_clientFactory.GetCheckpointStoreClient(),
options.EventProcessorOptions.RetryOptions.ToRetryPolicy(),
factoryContext.Descriptor.Id,
_logger);
_loggerFactory.CreateLogger<BlobsCheckpointStore>());

IListener listener = new EventHubListener(
factoryContext.Descriptor.Id,
Expand All @@ -69,7 +69,7 @@ public Task<ITriggerBinding> TryCreateAsync(TriggerBindingProviderContext contex
_clientFactory.GetEventHubConsumerClient(attribute.EventHubName, attribute.Connection, attribute.ConsumerGroup),
checkpointStore,
options,
_logger);
_loggerFactory);
return Task.FromResult(listener);
};

Expand Down
8 changes: 6 additions & 2 deletions sdk/eventhub/Microsoft.Azure.WebJobs.Extensions.EventHubs/tests/EventHubEndToEndTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ public async Task EventHub_PocoBinding()
}

var logs = host.GetTestLoggerProvider().GetAllLogMessages().Select(p => p.FormattedMessage);

CollectionAssert.Contains(logs, $"PocoValues(foo,data)");

var categories = host.GetTestLoggerProvider().GetAllLogMessages().Select(p => p.Category);
CollectionAssert.Contains(categories, "Microsoft.Azure.WebJobs.EventHubs.EventHubListener.EventProcessor");
}

[Test]
Expand All @@ -72,8 +74,10 @@ public async Task EventHub_StringBinding()
Assert.True(result);

var logs = host.GetTestLoggerProvider().GetAllLogMessages().Select(p => p.FormattedMessage);

CollectionAssert.Contains(logs, $"Input(data)");

var categories = host.GetTestLoggerProvider().GetAllLogMessages().Select(p => p.Category);
CollectionAssert.Contains(categories, "Microsoft.Azure.WebJobs.EventHubs.EventHubListener.EventProcessor");
}
}

Expand Down
3 changes: 1 addition & 2 deletions sdk/eventhub/Microsoft.Azure.WebJobs.Extensions.EventHubs/tests/EventHubListenerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ public void GetMonitor_ReturnsExpectedValue()
var functionId = "FunctionId";
var eventHubName = "EventHubName";
var consumerGroup = "ConsumerGroup";
var testLogger = new TestLogger("Test");
var host = new EventProcessorHost(consumerGroup,
"Endpoint=sb://test.servicebus.windows.net/;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=abc123=",
eventHubName,
Expand All @@ -224,7 +223,7 @@ public void GetMonitor_ReturnsExpectedValue()
consumerClientMock.Object,
Mock.Of<BlobsCheckpointStore>(),
new EventHubOptions(),
testLogger);
Mock.Of<LoggerFactory>());

IScaleMonitor scaleMonitor = listener.GetMonitor();

Expand Down
2 changes: 1 addition & 1 deletion ...rosoft.Azure.WebJobs.Extensions.Storage.Common/src/Shared/Queues/QueueProcessorOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal QueueProcessorOptions(QueueClient queue, ILoggerFactory loggerFactory,
{
Queue = queue ?? throw new ArgumentNullException(nameof(queue));
PoisonQueue = poisonQueue;
Logger = loggerFactory?.CreateLogger(LogCategories.CreateTriggerCategory("Queue"));
Logger = loggerFactory?.CreateLogger<QueueProcessor>();
Copy link
Member Author

Choose a reason for hiding this comment

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

Intentionally used QueueProcessor rather than QueueProcessorOptions as that fits better with the usage.

Options = options.Clone();
}

Expand Down
9 changes: 8 additions & 1 deletion sdk/storage/Microsoft.Azure.WebJobs.Extensions.Storage.Queues/tests/QueueProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.Azure.WebJobs.Host.Queues;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Moq;
using NUnit.Framework;

Expand Down Expand Up @@ -111,7 +112,10 @@ public async Task CompleteProcessingMessageAsync_FailureWithoutPoisonQueue_DoesN
[Test]
public async Task CompleteProcessingMessageAsync_MaxDequeueCountExceeded_MovesMessageToPoisonQueue()
{
QueueProcessorOptions context = new QueueProcessorOptions(_queue, null, _queuesOptions, _poisonQueue);
ILoggerFactory loggerFactory = new LoggerFactory();
var provider = new TestLoggerProvider();
loggerFactory.AddProvider(provider);
QueueProcessorOptions context = new QueueProcessorOptions(_queue, loggerFactory, _queuesOptions, _poisonQueue);
QueueProcessor localProcessor = new QueueProcessor(context);

bool poisonMessageHandlerCalled = false;
Expand Down Expand Up @@ -141,6 +145,9 @@ public async Task CompleteProcessingMessageAsync_MaxDequeueCountExceeded_MovesMe
Assert.NotNull(poisonMessage);
Assert.AreEqual(messageContent, poisonMessage.MessageText);
Assert.True(poisonMessageHandlerCalled);

var categories = provider.GetAllLogMessages().Select(p => p.Category);
CollectionAssert.Contains(categories, "Microsoft.Azure.WebJobs.Host.Queues.QueueProcessor");
}

[Test]
Expand Down