apm-agent-dotnet
apm-agent-dotnet copied to clipboard
No way to set `Code` property on captured exception
Is your feature request related to a problem? Please describe.
According to the specification, code
field is the error code set when the error happened, e.g. database error code. Not all databases and drivers return the error code when error occurred, however, in the case of MongoDB error such code presents and can be accessed from MongoCommandException. Unfortunately, in the current agent codebase is impossible to set such property.
Describe the solution you'd like
I don't have any exact suggestions right now, however, I see two possible ways how it can be achieved:
- Extend
CaptureError
andCaptureException
contracts to accept additional argumentcode
. It will solve issue which is addressed here, however, in the future, it will look like hell if method contracts will be extended for every custom property. - Extend
CaptureError
andCaptureException
contracts to return captured error instance. It's a more flexible way which allows in the future to also override other fields, however, at the moment it needs more work:- extend
IError
interface with additional fields - introduce an interface for
CapturedException
class
- extend
Describe alternatives you've considered
At the moment, only a reflection is an option, which is not very safe and good.
Additional context
The same is applicable for customizing Message
field of the CapturedException
class. MongoCommandExceptin
has ErrorMessage
property which contains a more detailed error message from MongoDB.
update: IError
interface was extended and CapturedException
, CapturedStackFrame
classes became public. So, the only thing is left: Extend CaptureError and CaptureException contracts to return captured error instance.
@gregkalapos, I know that it's a breaking change and all custom implementations need to change codebase, but it looks like the most flexible approach.
Hey, @gregkalapos
I'm returning back to this project and going on my previous issues.
After digging into the codebase, I know the actual reason why returning IError
interface isn't a good idea. The reason in queuing such error into the payload sender and send process isn't deterministic, which means that modification of the returned IError
instance can be not sent into the APM server.
But another possible solution came to mind (also has some limitations at the moment). We can introduce a new method CaptureError
which will accept the instance of IError
interface. It will allow customizing how errors need to be looked for various dependencies. However, in such case, it would be nice to make StacktraceHelper
as public. WDYT?
@vhatsura - wouldn't it help to just add an optional parameter to the CaptureError
and CaptureException
methods? Can't you set it when the error is capture?
Also fyi this branch contains slightly related POC which extends the error capturing to automatically capture logs on error level as error (the last 3 commits on the PR are the ones implementing it). - It comes from this issue and in that case I also just added optional parameters. I know the parameter explosion is not the nicest, but very few things can go wrong with that.
@vhatsura - wouldn't it help to just add an optional parameter to the
CaptureError
andCaptureException
methods? Can't you set it when the error is capture?Also fyi this branch contains slightly related POC which extends the error capturing to automatically capture logs on error level as error (the last 3 commits on the PR are the ones implementing it). - It comes from this issue and in that case I also just added optional parameters. I know the parameter explosion is not the nicest, but very few things can go wrong with that.
Optional parameters can help, but I afraid of the growing amount of parameters. Also, each new optional parameter will break binary compatibility with preserving the source compatibility.