feat: added changes to enable tracing in lambdas. #3554
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…m-aws-github-runner into nav/enable-tracing
…m-aws-github-runner into nav/enable-tracing
npalm
reviewed
Oct 25, 2023
npalm
reviewed
Oct 25, 2023
npalm
reviewed
Oct 25, 2023
npalm
reviewed
Oct 25, 2023
npalm
reviewed
Oct 25, 2023
…m-aws-github-runner into nav/enable-tracing
…m-aws-github-runner into nav/enable-tracing
…m-aws-github-runner into nav/enable-tracing
npalm
requested changes
Oct 31, 2023
There was a problem hiding this comment.
@GuptaNavdeep1983 nice work. Made some comments. If you like we can pair next week to get them resolved.
I also will try to run some tests later this week.
|
@npalm are these comments blocker to the PR? Can we not address them separately? |
npalm
reviewed
Nov 3, 2023
npalm
reviewed
Nov 3, 2023
npalm
reviewed
Nov 3, 2023
…m-aws-github-runner into nav/enable-tracing
npalm
reviewed
Nov 6, 2023
npalm
reviewed
Nov 6, 2023
|
Notes about testing: Testing deployment of main -> updateing to PR -> enable tracing
|
npalm
approved these changes
Nov 8, 2023
GuptaNavdeep1983
pushed a commit
that referenced
this pull request
Nov 8, 2023
🤖 I have created a release *beep* *boop* --- ## [5.4.0](v5.3.0...v5.4.0) (2023-11-08) ### Features * added changes to enable tracing in lambdas. ([#3554](#3554)) ([970e8a6](970e8a6)) ### Bug Fixes * **lambda:** Bump the aws group in /lambdas with 5 updates ([#3595](#3595)) ([581a4bf](581a4bf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
npalm
added a commit
that referenced
this pull request
Aug 7, 2024
## Problem Updating octkit auth app to 6.1.1. fails runtime. Not in our tests due to most of the SDK layer is mocked. Only information we catch is a generic Error. ``` HttpError: response.json(...).catch is not a function ``` Via [issue](octokit/auth-app.js#634) @wolfy1339 provides feedback that the cause could be related to overriding the fetch in octokit. In PR #3554 where tracing was introduced also the fetch command was overriden, this due the fact the node fetch was not working properly for tracing HTTP calls, which are in our case the cals to GitHub. This problem seems now to be fixed in the [AWS xray sdk](https://github.com/aws/aws-xray-sdk-node/blob/master/CHANGELOG.md#370). ## Solution The solution is quite simple. Just remove axios as fetch implementation (override) and use the default provide by octokit. ## Testing The problem was not detected in the unit tests. Reason is that the unit test mocking the octokit SDK. In case of an ovrride we have doen. We hsould have added a test using nock to mock the HTTP calls and not the full SDK to catch problems like this in the unit test. By removing the override, we can relay again on the test efforts done by octokit. ## References - fix #3966
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the need to enable tracing for the lambdas used in the runners architecture:
Highlights:
Additions:
Options:
Use Cloudwatch config agent which now supports to capture traces (link) can be used to capture traces and link them to the log groups.