newrelic / newrelic-java-agent

The New Relic Java agent
Apache License 2.0
202 stars 143 forks source link

Add ZIO 2 support #1106

Closed duartesaraiva98 closed 1 year ago

duartesaraiva98 commented 1 year ago

Overview

Adding instrumentation for ZIO 2 to the agent. Currently there is only instrumentation for ZIO 1 which is not compatible with ZIO 2. The majority of this code is based on ZIO 1 implementation with some slight changes to make it work.

Background for why this change is needed.

When we migrated to ZIO 2 we noticed some issues in the transactions data.

Problem 1 - Segments in Web Transactions do not get child segments

Stack:

What we currently experience is that the web-transactions that call on ZIO methods don’t track child segments. In the example of the image below, the tree of segments should be ”command-call-1” as the root segment and two child segments ”test-pg-call-1” and ”test-pg-call-2”. As you can see, only the root segment (”command-call-1”) is reported back.

zio2-web-example

Problem 2 - Non-web transactions are not started

In ZIO 2:

Stack:

What we currently experience is that non-web transactions are started but not linked to the segments inside the method that starts the transaction. In the example of the image below, the tree of segments should be ”command-call-1” as the root segment and two child segments ”test-pg-call-1” and ”test- pg-call-2”. The reality is that none of the segments show up and the transaction is ”completed” in just a few milliseconds.

zio2-non-web

In ZIO 1:

Stack:

For the non-web transactions, we saw the same issue happening in ZIO 1.x. The transaction exists but completes in just a few milliseconds and is not linked to any segments. Even when using the newrelic-scala-zio-api.

zio1-non-web

Expected Result for both Non-Web and Web Transactions

Stack:

What we expect for both non-web and web transactions can be seen in the image below. This is the behavior that we have always seen with new relic, even before we used ZIO. In this example, we were using ZIO 1.x in combination with new relic’s instrumentation for ZIO, which is only compatible with ZIO 1.x. In this example you can see the root segment ”command-call-1” as well as the two child segments ”test-pg-call-1” and ”test-pg-call-2”

zio1-web-tx

Related Github Issue

[Not applicable]

Checks

[Yes ] Are your contributions backwards compatible with relevant frameworks and APIs? [ No] Does your code contain any breaking changes? Please describe. [ No] Does your code introduce any new dependencies? Please describe.

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

duartesaraiva98 commented 1 year ago

@tbradellis Is this something you could take a look at? You added instrumentation for ZIO 1, right?

kford-newrelic commented 1 year ago

@duartesaraiva98 thanks for the PR!

We've got a fair bit of work on our board at the moment but we'll review this as soon as possible

kford-newrelic commented 1 year ago

@duartesaraiva98 apologies but we had to bump this out to the Apr-Jun quarter - we're over-capacity for the current quarter

tbradellis commented 1 year ago

We are closing this for now, because we are not able to get it on the roadmap to investigate the issue with losing Transaction context described in the comment here.

Feel free to resubmit with that addressed and we can likely get it merged. Tests were otherwise passing for the PR.

kford-newrelic commented 1 year ago

Reviewing for possible further action in the oct-dec quarter

kford-newrelic commented 1 year ago

Not able to address fixing the lost transaction issue in the coming oct-dec quarter. Will consider again for the following quarter.

jasonjkeller commented 12 months ago

Here's another repro app, this one from NR-175811 using ZIO 1.x: NewrelicTestProject.zip