The Wayback Machine - https://web.archive.org/web/20201224090759/https://github.com/elastic/apm-agent-dotnet/issues/379
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

Capture request body in ASP.NET Full Framework #379

Open
SergeyKleyman opened this issue Jul 17, 2019 · 6 comments
Open

Capture request body in ASP.NET Full Framework #379

SergeyKleyman opened this issue Jul 17, 2019 · 6 comments

Comments

@SergeyKleyman
Copy link
Contributor

@SergeyKleyman SergeyKleyman commented Jul 17, 2019

Derived from #213,

@katzdan
Copy link
Contributor

@katzdan katzdan commented Jul 17, 2019

Hi @SergeyKleyman ,

I can start working on this one. I might need some guidance though.
I'm assuming that the implementation should be identical to what's written in the docs (https://www.elastic.co/guide/en/apm/agent/nodejs/current/configuration.html#capture-body).

@SergeyKleyman
Copy link
Contributor Author

@SergeyKleyman SergeyKleyman commented Jul 17, 2019

Here are the links to all other Elastic APM agents that already have this feature implemented (I've taken them from Elastic APM Backend Agent Config Comparison spreadsheet)

I would recommend to start with the requirements for this feature by going over other agents' documentation for this feature to make sure requirements for .NET agent align with the other agents.

@gregkalapos
Copy link
Collaborator

@gregkalapos gregkalapos commented Jul 17, 2019

@katzdan it's totally fine if you start with this, just some thoughts:

Our ASP.NET Core part is more advanced, and the ASP.NET Full Framework project is not yet released. So #213 is maybe a better candidate to make it into production sooner - also writing tests is I think simpler for that case - although for Full Framework we already also have a nice setup, but it maybe needs more learning, the ASP.NET Core tests are simpler in-process tests.

@katzdan
Copy link
Contributor

@katzdan katzdan commented Jul 17, 2019

I'll start with #213 then. Thanks.

@andrewharry
Copy link

@andrewharry andrewharry commented Jul 25, 2019

Can you consider including redacting requirement? I often have business requirements to ensure sensitive data such as credit cards are not accidentally logged.

This could be as simple as some kind of regex replacement...

@gregkalapos
Copy link
Collaborator

@gregkalapos gregkalapos commented Jul 25, 2019

Can you consider including redacting requirement? I often have business requirements to ensure sensitive data such as credit cards are not accidentally logged.

This could be as simple as some kind of regex replacement...

@andrewharry thanks for the input.

That'd be covered by #300. We already have an ongoing PR by @katzdan. I'd say in the first round we won't do it - just to reduce the scope and make sure that at least some part of the feature lands soon. In #300 we'll come back to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.