-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
question about function instances #4
Comments
Hi @karpikpl thank you for thanking me :)! You are right to be concerned about this and yes, if you are using an HTTP trigger and have a decent # of requests, the global I considered this and I was careful not to let this cause an issue. I have a unit test making sure this exact thing scenario doesn't cause Technical explanation : It's true that it keeps creating a new console.log, but it's always creating the exact same one and in a way that wont 'nest' the reassignments, since it holds onto the original |
I see. |
It could, but would it matter? AFAIK If you want to help me test, make a function app with a n second cron function and a n*2 second cron function; they'll eventually step on each other after running for a bit. Feel free to upload the test project here, I'd really appreciate it! |
I'll try to test it this week. specifically:
hence, I was thinking that |
Wow that's an interesting find from their docs 👍 That text has been there since before I thought to make this package and I think it's incorrect, especially with regard to What I think they could be referring to in that paragraph, is the logs from the log stream (in the cloud CLI-looking window they show you when you invoke on the server, and in your local azure function emulator console). That's probably why they're referring to it as a trace output. I think the Microsoft docs just have a mistake in them. Here's a link to that document from January Also, I found this. It's actually more horrifying than just succumbing to passing around https://www.npmjs.com/package/applicationinsights#azure-functions-example I don't understand why Microsoft is spending so much energy ensuring logs for a single request stay together. AWS and GCP dont try to do this. The tools to query logs are incredibly powerful. Log something like a user Id or entity Id along with your data (which you probably need to do anyway to get meaningful logs). This covers 99% of use cases. |
I think your test proves what we suspected, that there will be overlap to some degree. I will perhaps add a disclaimer for this to my readme. At the very least it's good that it's documented in our chat! And just to confirm, you aren't able to get logs with just plain If you don't want to PR their repo then no problem, I will do it on your behalf. Just lmk! |
I forgot to tag you @karpikpl and also I updated my comment 15 minutes after editing it so the comment you got emailed from me had a stupid mistake in it! Sorry! :P |
Thanks for the reply @karpikpl and I'm glad you took a deep dive on this! And it's no problem at all! I suspected this could happen. And if not now, then in the future because why would Microsoft have such an awkward API if not for a feature like this so it was only a matter of time before someone found the precise leak in my abstraction :D To to clarify my question
I just want to make sure I'm not missing something and all of the sudden stdout is shipped to appinsights making my package deprecated--which is something that could happen in the future or it could literally be a checkbox somewhere in Azure functions. (Of course I would have to confirm this myself before doing anything about this!) Thanks again! |
module.exports = async function (context, myTimer) {
var timeStamp = new Date().toISOString();
if (myTimer.IsPastDue)
{
console.log('JavaScript is running late!');
}
console.log('Log statement one - XYZ', context.invocationId);
console.log('Log statement two - XYZ', context.invocationId);
// sleep for 1 s
await new Promise(resolve => setTimeout(resolve, 1000));
console.log('Log statement three - XYZ', context.invocationId);
}; |
Hi, thanks for putting this package together. So I just want to make sure i understand this correctly. We have a race condition here because, logging could be wrong in a scenario like this:
Actual Behavior Expected Behavior Am I missing something? |
That's exactly correct. I see no way around this, unfortunately. If you need deep application insights dependency tracing or log tracing, this package may not be the right choice and you may need to just pass down the My package is a semi-leaky abstraction just to evade passing the proprietary (If someone eventually figures out a way around this, I'm interested in adding it btw) |
Hi @BrianRosamilia, thanks for the quick response. I have run into this issue in the past running a standard nodejs server. In my research, I had seem some really amazing software out there that would allow us to do contextual logging by using an experimental feature in NodeJs called Async Hooks. The issue with that approach is that's it's experimental and could have an impact on performance as described here. I may try to take a stab at an implementation using a library like cls-hooked and somehow lookup console.log based on the current context that is being executed. I'll run some tests and check performance. Just thought I'd share the research i've found as this async problem has plagued me for years lol. I have also used dependency injection in the past, but that requires you to write your code and modules a certain way which is less than ideal. I love your simple approach and would like to find a similar simple solution. More reading here |
@joeyjiron06 very interesting. We can just put support for this behind an environment variable if you can get something working :) I appreciate your effort + communication!! |
Just to add my 2 cents. I also once tried to use a global logger in azure functions but the traces were not correctly associated to right function invocations. I also tried to use global setting for request-specific internationalization which ended up very badly. I was baffled as exactly the same approach worked in AWS Lambdas. Finally I found out that AWS Lambdas and Azure functions have totally different execution model: AWS Lambda: Use one execution context (node instance) for the full duration of function call. Do not process parallel requests using the same context, always launch a new context if there're no free contexts available. --> It's totally ok to use global variables, other requests won't change them during execution of one request. So I can succesfully use e.g. a global intance of pino-lambda logger that logs content and adds the correct aws request id for each log entry. Azure functions: Share the execution context i.e. serve multiple parallel requests using the same context. --> One just cannot use global variables. Only possibility to have log rows associated to correct invocation is to pass exectution context (or logger that wraps it) around as function arguments. That's what I ended up doing. It's ugly but the only way that really works. |
Hey @BrianRosamilia,
thanks for creating this package.
I was just wondering what would happen if azure function is called multiple times?
Could it re-use existing instance or is it always going to create new one?
Because if there's a chance of 2 (or more) triggers executing - wouldn't they override the context and global
console.log
?The text was updated successfully, but these errors were encountered: