Skip to content

TT-10153 Fix OTel memory leak#5587

Merged
tbuchaillot merged 7 commits into
masterfrom
fix/TT-10153-fix-otel-memory-leak
Oct 6, 2023
Merged

TT-10153 Fix OTel memory leak#5587
tbuchaillot merged 7 commits into
masterfrom
fix/TT-10153-fix-otel-memory-leak

Conversation

@tbuchaillot
Copy link
Copy Markdown
Contributor

@tbuchaillot tbuchaillot commented Oct 5, 2023 •
edited
Loading

Description

This PR updates the opentelemetry lib to the latest version (v0.0.19) which includes TykTechnologies/opentelemetry#32 and TykTechnologies/opentelemetry#30 fixes.

Related Issue

https://tyktech.atlassian.net/browse/TT-10153

Motivation and Context

https://tyktech.atlassian.net/browse/TT-10153

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@tbuchaillot tbuchaillot changed the title fix otel http handler multiple creation TT-10153 Fix OTel memory leak Oct 5, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 5, 2023 •
edited
Loading

API Changes

no api changes detected

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 5, 2023

PR Analysis

  • 🎯 Main theme: The PR is aimed at fixing a memory leak issue related to OpenTelemetry in the Gateway.
  • 📝 PR summary: The PR addresses a memory leak issue by modifying the OpenTelemetry HTTP handler in the Gateway. It also updates the version of the OpenTelemetry library used in the project.
  • 📌 Type of PR: Bug fix
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 2, The PR is relatively small and straightforward, focusing on a specific bug fix. However, understanding the impact of the changes requires some knowledge of OpenTelemetry.
  • 🔒 Security concerns: No

PR Feedback

  • 💡 General suggestions: It would be beneficial to include a brief explanation of how the changes fix the memory leak issue in the PR description. This would help reviewers understand the logic behind the changes and their expected impact.

  • 🤖 Code feedback:

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 5, 2023

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@buger
Copy link
Copy Markdown
Member

buger commented Oct 5, 2023

API tests result: success ✅
Branch used: refs/pull/5587/merge
Commit:
Triggered by: pull_request (@tbuchaillot)
Execution page

@buger
Copy link
Copy Markdown
Member

buger commented Oct 5, 2023

API tests result: success ✅
Branch used: refs/pull/5587/merge
Commit: 5b7f35f
Triggered by: pull_request (@tbuchaillot)
Execution page

@buger
Copy link
Copy Markdown
Member

buger commented Oct 5, 2023

API tests result: success ✅
Branch used: refs/pull/5587/merge
Commit: 8b7515b
Triggered by: pull_request (@tbuchaillot)
Execution page

@buger
Copy link
Copy Markdown
Member

buger commented Oct 5, 2023

API tests result: success ✅
Branch used: refs/pull/5587/merge
Commit: 6c0ee4a
Triggered by: pull_request (@tbuchaillot)
Execution page

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Oct 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@buger
Copy link
Copy Markdown
Member

buger commented Oct 6, 2023

API tests result: success ✅
Branch used: refs/pull/5587/merge
Commit: ad3a345
Triggered by: pull_request (@tbuchaillot)
Execution page

@tbuchaillot tbuchaillot merged commit f380a39 into master Oct 6, 2023
@tbuchaillot tbuchaillot deleted the fix/TT-10153-fix-otel-memory-leak branch October 6, 2023 08:49
@tbuchaillot
Copy link
Copy Markdown
Contributor Author

tbuchaillot commented Oct 6, 2023

/release to release-5.2

@tykbot
Copy link
Copy Markdown

tykbot Bot commented Oct 6, 2023

Working on it! Note that it can take a few minutes.

@buger buger mentioned this pull request Oct 6, 2023
Merged
7 tasks
tykbot Bot pushed a commit that referenced this pull request Oct 6, 2023
@tbuchaillot
TT-10153 Fix OTel memory leak (#5587)
9342ef2 
<!-- Provide a general summary of your changes in the Title above -->

<!-- Describe your changes in detail -->
This PR updates the opentelemetry lib to the latest version (v0.0.19)
which includes TykTechnologies/opentelemetry#32
and TykTechnologies/opentelemetry#30 fixes.

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
https://tyktech.atlassian.net/browse/TT-10153

<!-- Why is this change required? What problem does it solve? -->
https://tyktech.atlassian.net/browse/TT-10153

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit f380a39)
@tykbot
Copy link
Copy Markdown

tykbot Bot commented Oct 6, 2023

@tbuchaillot Succesfully merged PR

@tbuchaillot
Copy link
Copy Markdown
Contributor Author

tbuchaillot commented Oct 6, 2023

/release to release-5.2.1

@tykbot
Copy link
Copy Markdown

tykbot Bot commented Oct 6, 2023

Working on it! Note that it can take a few minutes.

@buger buger mentioned this pull request Oct 6, 2023
Merged
7 tasks
tykbot Bot pushed a commit that referenced this pull request Oct 6, 2023
@tbuchaillot
TT-10153 Fix OTel memory leak (#5587)
8842f86 
<!-- Provide a general summary of your changes in the Title above -->

<!-- Describe your changes in detail -->
This PR updates the opentelemetry lib to the latest version (v0.0.19)
which includes TykTechnologies/opentelemetry#32
and TykTechnologies/opentelemetry#30 fixes.

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
https://tyktech.atlassian.net/browse/TT-10153

<!-- Why is this change required? What problem does it solve? -->
https://tyktech.atlassian.net/browse/TT-10153

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit f380a39)
@tykbot
Copy link
Copy Markdown

tykbot Bot commented Oct 6, 2023

@tbuchaillot Succesfully merged PR

tbuchaillot added a commit that referenced this pull request Oct 6, 2023
@buger @tbuchaillot
Merging to release-5.2: TT-10153 Fix OTel memory leak (#5587) (#5596)
e61b7b0 
TT-10153 Fix OTel memory leak (#5587)

<!-- Provide a general summary of your changes in the Title above -->

## Description
<!-- Describe your changes in detail -->
This PR updates the opentelemetry lib to the latest version (v0.0.19)
which includes TykTechnologies/opentelemetry#32
and TykTechnologies/opentelemetry#30 fixes.
## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
https://tyktech.atlassian.net/browse/TT-10153
## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->
https://tyktech.atlassian.net/browse/TT-10153
## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

---------

Co-authored-by: Tomas Buchaillot <tombuchaillot89@gmail.com>
tbuchaillot added a commit that referenced this pull request Oct 6, 2023
@buger @tbuchaillot
Merging to release-5.2.1: TT-10153 Fix OTel memory leak (#5587) (#5597)
ffa83a2 
TT-10153 Fix OTel memory leak (#5587)

<!-- Provide a general summary of your changes in the Title above -->

## Description
<!-- Describe your changes in detail -->
This PR updates the opentelemetry lib to the latest version (v0.0.19)
which includes TykTechnologies/opentelemetry#32
and TykTechnologies/opentelemetry#30 fixes.
## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
https://tyktech.atlassian.net/browse/TT-10153
## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->
https://tyktech.atlassian.net/browse/TT-10153
## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

---------

Co-authored-by: Tomas Buchaillot <tombuchaillot89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mativm02 mativm02 mativm02 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tbuchaillot @buger @mativm02