Azure / azure-functions-eventhubs-extension

Event Hubs extension for Azure Functions
MIT License
20 stars 26 forks source link

Calculate approx time in queue for EventHubs messages and populate EventHub info #52

Closed lmolkova closed 4 years ago

lmolkova commented 4 years ago

This change enables custom metrics on time in queue for EventHubs messages and populates EventHub info on trigger request telemetry (this will allow to show AppMap properly and navigate to EventHub resource from AI views.

Please advise

  1. Is populating EventHub info on trigger details is a good idea Pros: all loggers can benefit from it Cons: no other triggers do it at least as of today and likely it's the same on all executions

Is there a better place?

  1. Extension already populates enqueued time in TriggerDetails, but for batches it populate time from 2 messages, while in AppInsights we really want all/average, so I propagate it within Activity, i.e. we have 2 different ways at the same time.

Can I change trigger details (e.g. send all enqueued times separated by comma)? Populate average?

This relies on https://github.com/Azure/azure-webjobs-sdk/pull/2428 changes in webjobs to populate data on AppInsights telemetry.

lmolkova commented 4 years ago

@alrod @brettsam can you please have a look and also help clarify couple of questions in the description?

lmolkova commented 4 years ago

@brettsam @alrod this one is ready for the review.

Tests are failing because they rely on changes in https://github.com/Azure/azure-webjobs-sdk/pull/2428. Is there a way to build current dev version of Microsoft.Azure.WebJobs.Logging.AppInsights to reference in here in the tests?

brettsam commented 4 years ago

I've just published 3.0.18-11717 to the azure-appservice repo (which is part of this repo's nuget.config). See if you can move to that and get tests passing.

lmolkova commented 4 years ago

@brettsam thanks a lot! Everything is green and ready to be merged if you don't have more feedback.

brettsam commented 4 years ago

@alrod -- I'd like you to take a look here since you're mostly the expert here. Don't want this to be a surprise :-)

lmolkova commented 4 years ago

@alrod can you please have a look?

lmolkova commented 4 years ago

@alrod can you please have a look?

ghost commented 4 years ago

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.