search

michaelhyatt / elastic-apm-mule4-agent

APM agent for tracing Mule 4 applications using Elastic APM
18 stars 9 forks source link

tracer.xml overridden error handling #4

Open ckho opened 4 years ago

ckho commented 4 years ago

In our current Mule application, we did a validation of value. If that does not pass, we will catch the error and then return a pair of error code and payload we set.

However, after importing the "tracer.xml", the error is overridden and becomes a 500 error.

image

Expected behaviour: image

Actual behaviour after importing "tracer.xml": image

michaelhyatt commented 4 years ago

Thanks for reporting it. What version are you using?

ckho commented 4 years ago

Mule: 4.1.6 elastic-apm-mule4-agent: 0.0.3 (built from distributed-http-tracing branch)

michaelhyatt commented 4 years ago

I built and tested it with 4.2.2, that branch is a bit of a dead-end where I experimented with few undocumented Mule APIs. Can you please test it with master?

ckho commented 4 years ago

Justed tested again with master and 4.1.6. The error is still overridden.

michaelhyatt commented 4 years ago

Can you try it on 4.2.2 please?

On Tue, 18 Feb 2020 at 4:40 pm, Chunkit Ho notifications@github.com wrote:

Justed tested again with master and 4.1.6. The error is still overridden.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/michaelhyatt/elastic-apm-mule4-agent/issues/4?email_source=notifications&email_token=ADXR5DPQM4NC6EKHTMQQOP3RDNYD5A5CNFSM4KWQLCVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMAU32Q#issuecomment-587288042, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXR5DNKGSB4ZEB4HQQPIULRDNYD5ANCNFSM4KWQLCVA .

-- Kind regards, Michael Hyatt

m: +614 4880 1078

ckho commented 4 years ago

Can you try it on 4.2.2 please?

Sorry I can't test it on 4.2.2, because my environment is limited by the PCE version. I can only use Mule runtime up to 4.1.x.

Is there any way to enable more log for debugging?

ckho commented 4 years ago

I just find out something. The issue exists only when the error is handled in a flow reference.

Mule: 4.1.6 elastic-apm-mule4-agent: 0.0.2 (master)

The one that works well: image

The one that doesn't work: image

ckho commented 4 years ago

One more finding: All error handling in all flows except the ones listening to the HTTP requests are omitted.

I do an experiment with 3 flow: Root flow: Listen to the http request. Call middle flow. Catch exception, create a variable "rootFlag" and return the value of all the flags. Middle flow: Call lower flow. Catch exception and create a variable "middleFlag". Lower flow: Do validation. Catch exception and create a variable "lowerFlag".

Only the error handling in root flow is executed.

michaelhyatt commented 4 years ago

Yep, I saw some irregularities in how exceptions are handled. I will take a look.

BTW, do you know if Mulesoft released anything after 4.1.6 and 4.2.2? Any news on 4.3?

On Tue, 18 Feb 2020 at 18:51, Chunkit Ho notifications@github.com wrote:

One more finding: All error handling in all flows except the ones listening to the HTTP requests are omitted.

I do an experiment with 3 flow: Root flow: Listen to the http request. Call middle flow. Catch exception, create a variable "rootFlag" and return the value of all the flags. Middle flow: Call lower flow. Catch exception and create a variable "middleFlag". Lower flow: Do validation. Catch exception and create a variable "lowerFlag".

Only the error handling in root flow is executed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/michaelhyatt/elastic-apm-mule4-agent/issues/4?email_source=notifications&email_token=ADXR5DIDIRNLXAU2VYEW5M3RDOHQDA5CNFSM4KWQLCVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMA6IFQ#issuecomment-587326486, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXR5DIH4ZCG42XC47KEGQLRDOHQDANCNFSM4KWQLCVA .

ckho commented 4 years ago

Not yet. Still 4.1.6 and 4.2.2. Seems like their release cycle is not that regular.

But Elastic looks like very regular. Almost every 2 months they have a new version On 19 Feb 2020, 6:53 AM +0800, Michael Hyatt notifications@github.com, wrote:

Yep, I saw some irregularities in how exceptions are handled. I will take a look.

BTW, do you know if Mulesoft released anything after 4.1.6 and 4.2.2? Any news on 4.3?

On Tue, 18 Feb 2020 at 18:51, Chunkit Ho notifications@github.com wrote:

One more finding: All error handling in all flows except the ones listening to the HTTP requests are omitted.

I do an experiment with 3 flow: Root flow: Listen to the http request. Call middle flow. Catch exception, create a variable "rootFlag" and return the value of all the flags. Middle flow: Call lower flow. Catch exception and create a variable "middleFlag". Lower flow: Do validation. Catch exception and create a variable "lowerFlag".

Only the error handling in root flow is executed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/michaelhyatt/elastic-apm-mule4-agent/issues/4?email_source=notifications&email_token=ADXR5DIDIRNLXAU2VYEW5M3RDOHQDA5CNFSM4KWQLCVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMA6IFQ#issuecomment-587326486, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXR5DIH4ZCG42XC47KEGQLRDOHQDANCNFSM4KWQLCVA .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ckho commented 4 years ago

I have drilled the code and done some experiment.

I found that if we add any hook, like exceptionally or thenApply, on action.proceed(), the issue would occur.

This is tested on a raw project without adding the apm-agent. I created a Mule project and then overrided both the ProcessorInterceptorFactory and ProcessorInterceptor.

michaelhyatt commented 4 years ago

Yes, agree, exception handling needs to be refined a bit further. I need to put together a quick design brief on how the exceptions should be handled within the APM agent: resuming the execution flow, capturing exception, etc.

michaelhyatt commented 4 years ago

The main issue with the notification API in Mule 4 is that there is no way now to mutate the Mule event in the notification. In Mule 3 I was able to mutate the event to add properties to propagate the distributed header. In Mule 4 it is completely changed. I was also experimenting with the new APIs to find a way not to store a thread-safe map of in-flight transactions and spans, but it seems to be the only way so far. I was working on other things hoping MuleSoft will release 4.3 and finally update the documentation for the runtime APIs, but it hasn't happened yet.

On Wed, 11 Mar 2020 at 22:12, Chunkit Ho notifications@github.com wrote:

I see that notification API is used in mule3-agent.

It looks like the API is still available in Mule 4. Can the tracing be implemented using that? Was there any reason changed the implementation way?

https://www.mulesoft.org/docs/site/4.1.1/apidocs/org/mule/runtime/api/notification/NotificationListener.html

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/michaelhyatt/elastic-apm-mule4-agent/issues/4?email_source=notifications&email_token=ADXR5DPOF5XJ6S2OOUUW7DLRG5W23A5CNFSM4KWQLCVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOPD77Y#issuecomment-597573631, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXR5DKQRNLV5QSQYLO25OLRG5W23ANCNFSM4KWQLCVA .