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.

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

observability of kiota clients #618

Closed
Tracked by #1049
nikithauc opened this issue Sep 13, 2021 · 6 comments · Fixed by #1841
Closed
Tracked by #1049

observability of kiota clients #618

nikithauc opened this issue Sep 13, 2021 · 6 comments · Fixed by #1841
Assignees
Labels
CLI Work to support generating CLIs with Kiota Csharp Pull requests that update .net code enhancement New feature or request fixed generator Issues or improvements relater to generation capabilities. Go Java PHP Python Ruby TypeScript Pull requests that update Javascript code

Comments

@nikithauc
Copy link
Contributor

nikithauc commented Sep 13, 2021

AB#11127

@nikithauc nikithauc added the enhancement New feature or request label Sep 13, 2021
@baywet baywet added Csharp Pull requests that update .net code Java PHP Python Ruby TypeScript Pull requests that update Javascript code labels Sep 14, 2021
@baywet
Copy link
Member

baywet commented Sep 14, 2021

Thanks for logging this one. This is something I had put on ice for a while.
I believe that only the core implementations (serialization, http, auth) need to log things. I don't believe the generated result needs to log anything itself per se. But I'm open to perspectives on this one. And I'd also like @darrelmiller input.

We could also provide a logging middleware with the default http core implementation. But it feels like giving people a gun to shoot themselves in the foot with. This could easily lead to logged tokens, or personal information, logs which could be compromised as we've seen in the past.

I do think we should seriously consider open telemetry

@baywet baywet added CLI Work to support generating CLIs with Kiota Go labels Nov 29, 2021
@darrelmiller
Copy link
Member

Yes https://opentelemetry.io/ is what we should be looking at. We should also follow the Azure SDK logging policies https://azure.github.io/azure-sdk/general_azurecore.html#logging-policy unless we have good reason not to.

Ideally we want people to be able to reuse this OpenTelemetry exporter for Azure Monitor https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/README.md

@baywet
Copy link
Member

baywet commented Nov 30, 2021

Thanks for this additional precision. Where would you have logging implemented? Abstractions? http? auth? serialization? all of them?
I don't think the logging should be added to what we generate, but only in the "handcrafted" code.

@darrelmiller
Copy link
Member

I don't think the logging should be added to what we generate, but only in the "handcrafted" code.
Agreed.

Abstractions - I don't think so.
Http - definitely
Auth - definitely
Serialization - maybe.

@baywet baywet added the generator Issues or improvements relater to generation capabilities. label Jun 23, 2022
@baywet baywet assigned darrelmiller and unassigned baywet Jun 23, 2022
@baywet baywet added the Python label Aug 18, 2022
@baywet baywet changed the title Logging in Kiota observability of kiota clients Sep 9, 2022
@baywet baywet assigned baywet and unassigned darrelmiller Sep 9, 2022
@baywet
Copy link
Member

baywet commented Sep 9, 2022

spec in progress microsoftgraph/msgraph-sdk-design#72

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Work to support generating CLIs with Kiota Csharp Pull requests that update .net code enhancement New feature or request fixed generator Issues or improvements relater to generation capabilities. Go Java PHP Python Ruby TypeScript Pull requests that update Javascript code
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants