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

fix(azure-functions): redirect console to azure context for logging #542

Closed

mcremer-able wants to merge 93 commits into nitrojs:main from mcremer-able:main

Contributor

mcremer-able commented Oct 4, 2022

🔗 Linked issue

❓ Type of change

📖 Documentation (updates to the documentation or readme)
🐞 Bug fix (a non-breaking change that fixes an issue)
👌 Enhancement (improving an existing functionality like performance)
✨ New feature (a non-breaking change that adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

At the moment there is no easy way to run nitro in an azure-funtions context and retain the ability to log out.
This pr redirects the built in console.log(),console.warn(),console.info() and console.error() to the azure provided logging context.
This adresses #383 ,but should be refined in the future to allow a more flexible log target

📝 Checklist

I have linked an issue or discussion.
I have updated the documentation accordingly.


          add a logging interceptor for azure context

a1c7bbb

vercel bot deployed to Preview

October 4, 2022 08:59

View deployment

danielroe reviewed

View reviewed changes

src/runtime/entries/azure-functions.ts

               export async function handle (context, req) {
                 const url = '/' + (req.params.url || '')
+                interceptLogging(context)

Member

danielroe Oct 4, 2022

Will this have issues with logs being associated to the wrong request if there are multiple parallel requests?

If not (that is, if there is only one aggregated logging bucket), then do we need to run this on every incoming request or could we early return if the interceptor has already been run?

Contributor Author

mcremer-able Oct 6, 2022

As I understand it, it is a single logging bucket for the whole function app.

We could optimize and dont intercept ,if we are already intercepting.
The gains do not outweight the potentiell bugs, in my opinion, with the added complexity we get.

I rather intercept twice than miss a critical log.


          Don't intercept ,if we are already intercepting

e28b891

vercel bot deployed to Preview

October 13, 2022 08:53

View deployment

mcremer-able requested a review from danielroe

October 24, 2022 09:15

Member

danielroe commented Oct 24, 2022

Regarding my comment:

Will this have issues with logs being associated to the wrong request if there are multiple parallel requests?

I'm checking with the Azure team to ensure that there are no downsides to this approach.

pi0 changed the title ~~fix[azure-functions]: redirect console to azure context for logging~~ fix(azure-functions): redirect console to azure context for logging

alvarlagerlof and others added 22 commits

January 9, 2023 09:22


          docs: fix readme grammar (nitrojs#813)

660acd1


          docs: fix deprecated function in routing example (nitrojs#816)

a1b2db9

Changed deprecated ```useBody()``` to ```readBody()```.

fixes: nitrojs#789


          docs: add new render deployment example & steps (nitrojs#811)

d1b0cfb


          fix(build): do not override publicAssets (nitrojs#817)

a3492b0


          feat(dev): support for /_vfs.json (nitrojs#809)

3f8ca7a


          chore(deps): update devdependency @types/fs-extra to v11 (nitrojs#814)

d256487

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(deps): update all non-major dependencies (nitrojs#810)

45b3abd

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(dev): improve vfs ui (nitrojs#802)

dcc59c7


          chore(deps): update dependency mlly to ^1.1.0 (nitrojs#820)

976d81d

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix: resolve and include scanDirs within node_modules for auto impo…

4654e59

…rt (nitrojs#812)

Co-authored-by: Pooya Parsa <[email protected]>


          style: lint code

15edef0


          chore(deps): update all non-major dependencies (nitrojs#824)

e12dae1

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          refactor: update event. to event.node. (nitrojs#828)

5d5b62e


          fix(netlify, vercel): explicit server rendering with disabled cache/s…

bece791

…wr (nitrojs#829)


          docs: fix typo in code block (nitrojs#830)

84a65bc


          chore(deps): update all non-major dependencies to ^0.27.1 (nitrojs#831)

ae708f0

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(deps): update all non-major dependencies (nitrojs#832)

8194ee4

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          feat(netlify): use esm entrypoint (nitrojs#833)

c1df65e


          chore: add lint fix command shortcut (nitrojs#835)

0211e60


          fix(prerender): check link's pathname only for extensions (nitrojs#791)

6585e8d


          fix(rumtime): disable server-timing header via options.timing (nitroj…

ed3a480

…s#823)


          fix: avoid using file urls for normalized paths in options

nitrojs#836

manh-gntvn and others added 17 commits

January 30, 2023 12:52


          fix(prerender): encode urls passed to local fetch (nitrojs#891)

930438a


          chore: update unenv

b77f082


          chore: format code

50e1a8f


          chore(release): v2.1.1

af0409f


          fix(types): allow narrowing of the method option in `NitroFetchOption…

a12e1e7

…s` (nitrojs#883)

* feat(fetch): allow for futher narrowing of the method option

* style: simplify fetch options type


          chore(deps): update devdependency typescript to ^4.9.5 (nitrojs#895)

40c21a2

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(vercel-edge): add temporary workaround for ‍process.cwd‍ usage (n…

c784b87

…itrojs#898)


          fix(dev): don't overwrite proxy headers if already set (nitrojs#896)

f7eed65

* fix: don't overwrite proxy headers if already set

* perf: only call `getHeaders()` once

---------

Co-authored-by: Mike Bellika <[email protected]>


          fix: provide fallback string values for undefined runtimeConfig (ni…

881a0ba

…trojs#907)


          docs: fix heroku typo (nitrojs#900)

325e919

Typo on the Heroku deployment page


          docs: fix stormkit images path (nitrojs#903)

10a7ee7


          fix(vercel): allow non-glob cache rules to apply to / (nitrojs#908)

4d89e31


          chore: update dependencies

dbb89a1


          fix(externals): use stable dependency tree (nitrojs#909)

78dd4f9

* refactor: improve code readability

* implement stable tree


          docs: fix path for server assets (nitrojs#917)

a899f1d


          fix(dev): mount src and root as read-only by default (nitrojs#920)

a459e1e


          chore(deps): update all non-major dependencies (nitrojs#911)

41884ba

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Member

pi0 commented Feb 6, 2023

Any updates on this?


          spread params

c45e4c9

vercel bot deployed to Preview

February 6, 2023 15:23

View deployment

mcremer-able added 4 commits

February 6, 2023 16:35


          add a logging interceptor for azure context

e1e0114


          Don't intercept ,if we are already intercepting

1068b4e


          spread params

f5c9b36


          Merge branch 'main' of https://github.com/mcremer-able/nitro

c277c76

vercel bot deployed to Preview

February 6, 2023 15:38

View deployment

Contributor Author

mcremer-able commented Feb 6, 2023

ah F thats not what I wanted
Rebase was the wrong command to fetch upstream changes
Any git wizards that can help?

About the concerns that we associate logs of different requests with each other.
The Microsoft docs say that context.log does disriminate based on request, but console.log does not
see docs

Member

danielroe commented Feb 6, 2023

In that case I think we need a different approach, almost certainly not overwriting console.log but instead exposing azure utilities on event.context.azure, much like we are discussing doing with cloudflare.

Contributor Author

mcremer-able commented Feb 7, 2023

I will close this as the long-term option will be an event.context.azure object with the necesarry properties.

For the short term I will continue to patch nitro in my CI with the console.log interceptor

mcremer-able closed this

Contributor

tobiasdiez commented Feb 7, 2023

I agree that event.context.azure (or a more general purpose event.context.log) is a good idea and solves the issue for manual log statements. However, often a library that one uses logs output via console.log and this output is simply lost when deployed to azure, making it really hard to debug/investigate issues. Thus, there should also be a way to redirect output to the correct logger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

26 participants