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

Add instrumentation for r2dbc-postgresql-0.9.2 #1410

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

koenpunt
Copy link
Contributor

@koenpunt koenpunt commented Jul 24, 2023

Before contributing, please read our contributing guidelines and code of conduct.

Overview

Adds updated instrumentation for r2dbc-postgresql-0.9.2, since instrumentation was disabled in #999.

Related Github Issue

Include a link to the related GitHub issue, if applicable

Testing

The agent includes a suite of tests which should be used to
verify your changes don't break existing functionality. These tests will run with
Github Actions when a pull request is made. More details on running the tests locally can be found
here,

Checks

  • Your contributions are backwards compatible with relevant frameworks and APIs.
  • Your code does not contain any breaking changes. Otherwise please describe.
  • Your code does not introduce any new dependencies. Otherwise please describe.

@CLAassistant
Copy link

CLAassistant commented Jul 24, 2023

CLA assistant check
All committers have signed the CLA.

@koenpunt
Copy link
Contributor Author

I tried running the tests locally, but couldn't get that to work, and was hoping to get the workflow running here on CI, but I see that maintainer requires approval.

@koenpunt
Copy link
Contributor Author

koenpunt commented Jul 24, 2023

Okay, got the tests to run by not running gradle with the --parallel flag. Also had to update the EmbeddedPostgres usage to assign a free port to not conflict with my local postgres installation.

@koenpunt koenpunt marked this pull request as ready for review July 24, 2023 15:10
@kford-newrelic kford-newrelic added the estimate Issue needing estimation label Aug 9, 2023
@kford-newrelic kford-newrelic added oct-dec qtr Represents proposed work item for the Oct-Dec quarter 3 Story Point Estimate and removed estimate Issue needing estimation labels Sep 20, 2023
@meiao meiao self-assigned this Oct 16, 2023
@meiao meiao mentioned this pull request Oct 16, 2023
@meiao
Copy link
Contributor

meiao commented Oct 18, 2023

I've encountered some problems with segments not ending properly.
Investigating.

UPDATE: The issue already exists in the instrumentation for previous versions of the instrumentation and seems only to affect Connection creationg.

@koenpunt
Copy link
Contributor Author

I've encountered some problems with segments not ending properly.

We've been experiencing the same, also the reason newrelic is currently unusable for, and thus has been disabled for the past month already.

@meiao meiao merged commit 4d725c0 into newrelic:main Oct 19, 2023
@meiao
Copy link
Contributor

meiao commented Oct 19, 2023

The segments that were not ended were related with the creation of Connections.
We will open a different ticket to investigate that.

@koenpunt did you encounter something different?

@koenpunt
Copy link
Contributor Author

We didn't investigate too much in what the problem was, also because we didn't really know where to look. What didn't help was the noise the token linker produces in the traces/profiling.

@koenpunt
Copy link
Contributor Author

Also; we've been using it with v1.0.0 of the r2dbc library, not sure if there it's because of the same reasons?

@meiao
Copy link
Contributor

meiao commented Oct 20, 2023

The issue that I found is the same across all versions.

@koenpunt koenpunt deleted the r2dbc-postgresql-0.9.2 branch October 20, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 Story Point Estimate oct-dec qtr Represents proposed work item for the Oct-Dec quarter
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants