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.

Sign up for GitHub

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

Jump to bottom

Merging to release-5.2: TT-10153 Fix OTel memory leak (#5587) #5596

Merged
merged 2 commits into from
Oct 6, 2023
Merged

Merging to release-5.2: TT-10153 Fix OTel memory leak (#5587) #5596

merged 2 commits into from
Oct 6, 2023

Conversation

buger
Copy link
Member

@buger buger commented Oct 6, 2023

TT-10153 Fix OTel memory leak (#5587)

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
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)
@tbuchaillot
fix conflicts
2cc760b
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

PR Analysis

  • 🎯 Main theme: This PR is about updating the opentelemetry library to the latest version to fix a memory leak issue.
  • 📝 PR summary: The PR updates the opentelemetry library to the latest version (v0.0.19) which includes fixes for a memory leak issue. The PR also updates the grpc library version and resolves conflicts in the go.mod file.
  • 📌 Type of PR: Bug fix
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 2, because the PR is straightforward and involves only version updates in the go.mod file.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR seems to be well-structured and the changes are clear. However, it would be beneficial to include information about how the memory leak was identified and how the update in the library version fixes it. This would provide more context to the reviewers.

  • 🤖 Code feedback:

    • relevant file: go.mod
      suggestion: Ensure that the updated versions of the libraries are compatible with the rest of the project dependencies to avoid potential conflicts or issues. [important]
      relevant line: github.com/TykTechnologies/opentelemetry v0.0.19

    • relevant file: go.mod
      suggestion: It's recommended to run tests after updating library versions to ensure that the update doesn't break existing functionality. [medium]
      relevant line: google.golang.org/grpc v1.58.0

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.

@tbuchaillot tbuchaillot marked this pull request as ready for review October 6, 2023 08:51
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

API Changes

no api changes detected

@tbuchaillot tbuchaillot enabled auto-merge (squash) October 6, 2023 08:59
@sonarcloud
Copy link

sonarcloud 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

@tbuchaillot tbuchaillot merged commit e61b7b0 into release-5.2 Oct 6, 2023
13 of 18 checks passed
@tbuchaillot tbuchaillot deleted the merge/release-5.2/f380a3934969816c95351e78648a36cada475f48 branch October 6, 2023 09:05
@buger
Copy link
Member Author

buger commented Oct 6, 2023

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

@buger
Copy link
Member Author

buger commented Oct 6, 2023

API tests result: failure 🚫
Branch used: refs/pull/5596/merge
Commit:
Triggered by: pull_request (@buger)
Execution page

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@buger @tbuchaillot