feat(codecov): Fetch code coverage for stacktrace #43116

jangjodi · 2023-01-11T18:26:13Z

Fixes WOR-2550

snigdhas · 2023-01-11T21:54:26Z

src/sentry/api/endpoints/project_stacktrace_link.py

+            CODECOV_URL.replace("service", config["provider"]["key"])
+            .replace("owner_username", repo_name[0])
+            .replace("repo_name", repo_name[1])


Try using variables and string interpolation here instead:

CODECOV_URL = "https://api.codecov.io/api/v2/{service}/{owner_username}/repos/{repo_name}/report"

Suggested change

CODECOV_URL.replace("service", config["provider"]["key"])

.replace("owner_username", repo_name[0])

.replace("repo_name", repo_name[1])

CODECOV_URL.format(service=config["provider"]["key"],owner_username=repo_name[0], repo_name=repo_name[1])

snigdhas · 2023-01-11T21:55:17Z

src/sentry/api/endpoints/project_stacktrace_link.py

+    def get_codecov_line_coverage(
+        self, config: JSONData, source_url: str, access_token: str
+    ) -> Any:
+        repo_name = config["repoName"].split("/")


You can assign both halves of the split to a variable, like so:

Suggested change

repo_name = config["repoName"].split("/")

owner_username, repo_name = config["repoName"].split("/")

snigdhas · 2023-01-11T21:57:45Z

src/sentry/api/endpoints/project_stacktrace_link.py

+        codecov_path_index = 0
+
+        # Extract path from URL from "blob/<sha_or_branch>/" onwards
+        for i, path in enumerate(source_url_split):


Not entirely sure what this logic is trying to do, but it looks like we're trying to find a specific part of the path to split at.

Would using a regular expression work better here?

An example of the logic we are trying to achieve is:

source_url = "https://github.com/getsentry/sentry/blob/8a0bf7246d39cdeb56d457134ee32767673b7492/src/sentry/shared_integrations/client/base.py"
but we want the path to be
src/sentry/shared_integrations/client/base.py"

Thank you for your suggestion, I used find to clean this part up

snigdhas · 2023-01-11T22:09:50Z

tests/sentry/api/endpoints/test_project_stacktrace_link.py

@@ -225,6 +228,35 @@ def test_file_no_stack_root_match(self, mock_integration):
        assert response.data["error"] == "stack_root_mismatch"
        assert response.data["integrations"] == [serialized_integration(self.integration)]

+    @responses.activate


What does this do?

This was a mistake that I removed in a previous commit, I'm not sure why it was still visible. I think it's gone now haha

snigdhas · 2023-01-11T22:22:23Z

src/sentry/api/endpoints/project_stacktrace_link.py

+
+    def get_codecov_line_coverage(
+        self, config: JSONData, source_url: str, access_token: str
+    ) -> Iterable[Union[Any, Optional[int]]]:


We probably want to use Tuple instead of Union, and Sequence instead of Iterable.

For the lineCoverage, how about something like Sequence[Sequence[int, int]]. Or we can define a type called LineCoverage as Sequence[Sequence[int, int]] and return Optional[LineCoverage] to make the following more readable.

Suggested change

) -> Iterable[Union[Any, Optional[int]]]:

) -> Tuple[Sequence[Sequence[int, int]], Optional[int]]:

Thank you for the suggestion! I did Tuple[Optional[Sequence[Tuple[int, int]]], just because Sequence can only take one parameter

Co-authored-by: Snigdha Sharma <snigdha.sharma@sentry.io>

snigdhas · 2023-01-12T19:08:00Z

src/sentry/api/endpoints/project_stacktrace_link.py

+            except Exception:
+                if status_code != 404:
+                    logger.exception("Failed to get coverage from Codecov")
+                line_coverage = None


Does this need to be reset this here, given the assignment on L202?

Good catch, thank you!

src/sentry/api/endpoints/project_stacktrace_link.py

…43143) Add the new fields from #43116 to the frontend WOR-2551

armenzg · 2023-01-18T14:57:52Z

src/sentry/integrations/utils/codecov.py

+CODECOV_TOKEN = options.get("codecov.client-secret")
+
+
+class Codecov:


I would not make it a class but up to you. Specially since you're not taking advantage of self.

src/sentry/options/defaults.py

armenzg · 2023-01-18T15:00:21Z

src/sentry/integrations/utils/codecov.py

@@ -0,0 +1,31 @@
+from typing import Optional, Sequence, Tuple


When adding new files check to see if the file is listed under mypy.ini

Thank you, this is good to know!

src/sentry/integrations/utils/codecov.py

src/sentry/api/endpoints/project_stacktrace_link.py

armenzg · 2023-01-18T15:23:28Z

src/sentry/api/endpoints/project_stacktrace_link.py

+                            branch=current_config["config"]["defaultBranch"],
+                            path=current_config["outcome"]["sourcePath"],
+                        )
+                    except Exception:


If we use raise_status(), this code would look more like this:

except requests.exceptions.HTTPError as error: if error.response.status_code == 404: logger.exception("Failed to get expected coverage data from Codecov, pending investigation. Continuing execution.") except Exception: logger.exception("Something unexpected happen. Continuing execution")

For now, let's error on everything and later on determine if we need to silence certain cases:

except Exception: logger.exception("Failed to get expected coverage data from Codecov, pending investigation. Continuing execution.")

You can remove the next few lines.

@armenzg

Co-authored-by: @armenzg <armenzg@sentry.io>

armenzg · 2023-01-18T17:44:03Z

src/sentry/api/endpoints/project_stacktrace_link.py

 from sentry.models import Integration, Project, RepositoryProjectPathConfig
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.utils.event_frames import munged_filename_and_frames
 from sentry.utils.json import JSONData

 logger = logging.getLogger(__name__)
+CODECOV_TOKEN = options.get("codecov.client-secret")


You can directly import CODECOV_TOKEN from sentry.integrations.utils.codecov and you will not need to do this here.

armenzg · 2023-01-18T17:45:04Z

src/sentry/api/endpoints/project_stacktrace_link.py

+                        if result["lineCoverage"]:
+                            result["codecovStatusCode"] = 200
+                    except requests.exceptions.HTTPError as error:
+                        result["codecovStatusCode"] = error.response.status_code


getsentry-bot · 2023-01-18T21:13:38Z

PR reverted: 05056a3

This reverts commit 3d2231d. Co-authored-by: armenzg <44410+armenzg@users.noreply.github.com>

@armenzg

Fixes WOR-2550 Co-authored-by: Snigdha Sharma <snigdha.sharma@sentry.io> Co-authored-by: @armenzg <armenzg@sentry.io>

@armenzg

Fixes WOR-2550 Co-authored-by: Snigdha Sharma <snigdha.sharma@sentry.io> Co-authored-by: @armenzg <armenzg@sentry.io>

feat(codecov): Add BE to fetch codecov line coverage

052579a

Fixes WOR-2550

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 11, 2023

vercel bot deployed to Preview – sentry January 11, 2023 18:28 View deployment

vercel bot deployed to Preview – storybook January 11, 2023 18:29 View deployment

chore: Add codecov status code to result

fa8d091

vercel bot deployed to Preview – sentry January 11, 2023 22:00 View deployment

vercel bot deployed to Preview – storybook January 11, 2023 22:01 View deployment

Merge branch 'master' into jodi/wor-2550

a85fffb

vercel bot deployed to Preview – sentry January 11, 2023 22:07 View deployment

vercel bot deployed to Preview – storybook January 11, 2023 22:07 View deployment

snigdhas reviewed Jan 11, 2023

View reviewed changes

snigdhas mentioned this pull request Jan 11, 2023

feat(codecov): Add new response fields for StacktraceLinkResult API #43143

Merged

vercel bot deployed to Preview – sentry January 11, 2023 23:21 View deployment

vercel bot deployed to Preview – storybook January 11, 2023 23:22 View deployment

jangjodi force-pushed the jodi/wor-2550 branch from 724c326 to b3c7612 Compare January 11, 2023 23:27

ref: Address PR comments

b1ffc8b

Co-authored-by: Snigdha Sharma <snigdha.sharma@sentry.io>

jangjodi force-pushed the jodi/wor-2550 branch from b3c7612 to b1ffc8b Compare January 11, 2023 23:30

vercel bot deployed to Preview – sentry January 11, 2023 23:33 View deployment

vercel bot deployed to Preview – storybook January 11, 2023 23:33 View deployment

jangjodi and others added 2 commits January 12, 2023 08:49

ref: Clean function return

8eb9106

Merge branch 'master' into jodi/wor-2550

7b76ee3

vercel bot deployed to Preview – storybook January 12, 2023 16:56 View deployment

vercel bot deployed to Preview – sentry January 12, 2023 16:57 View deployment

snigdhas reviewed Jan 12, 2023

View reviewed changes

jangjodi and others added 2 commits January 12, 2023 11:33

ref: Address PR comments

aae4cae

Merge branch 'master' into jodi/wor-2550

a90009b

vercel bot deployed to Preview – sentry January 12, 2023 19:38 View deployment

vercel bot deployed to Preview – storybook January 12, 2023 19:38 View deployment

snigdhas added a commit that referenced this pull request Jan 12, 2023

feat(codecov): Add new response fields for StacktraceLinkResult API (#…

b828179

…43143) Add the new fields from #43116 to the frontend WOR-2551

ref: Cleaned up code

91509f6

jangjodi requested a review from armenzg January 17, 2023 22:49

armenzg reviewed Jan 18, 2023

View reviewed changes

ref: Address PR comments

97b1072

Co-authored-by: @armenzg <armenzg@sentry.io>

jangjodi requested a review from a team as a code owner January 18, 2023 17:18

Merge branch 'master' into jodi/wor-2550

325cce7

jangjodi requested a review from armenzg January 18, 2023 17:20

vercel bot deployed to Preview – sentry January 18, 2023 17:22 View deployment

vercel bot deployed to Preview – storybook January 18, 2023 17:24 View deployment

fix: Fix typing

41be6c2

armenzg requested changes Jan 18, 2023

View reviewed changes

vercel bot deployed to Preview – sentry January 18, 2023 17:48 View deployment

ref: Add token import

e5a4495

jangjodi requested a review from armenzg January 18, 2023 17:49

vercel bot deployed to Preview – sentry January 18, 2023 17:51 View deployment

vercel bot deployed to Preview – storybook January 18, 2023 17:54 View deployment

armenzg approved these changes Jan 18, 2023

View reviewed changes

jangjodi and others added 2 commits January 18, 2023 12:12

fix: Fix tests to include token

a57c46e

Merge branch 'master' into jodi/wor-2550

df88c72

vercel bot deployed to Preview – sentry January 18, 2023 20:18 View deployment

vercel bot deployed to Preview – storybook January 18, 2023 20:21 View deployment

jangjodi merged commit 3d2231d into master Jan 18, 2023

jangjodi deleted the jodi/wor-2550 branch January 18, 2023 21:08

armenzg added the Trigger: Revert add to a merged PR to revert it (skips CI) label Jan 18, 2023

getsentry-bot added a commit that referenced this pull request Jan 18, 2023

Revert "feat(codecov): Fetch code coverage for stacktrace (#43116)"

05056a3

This reverts commit 3d2231d. Co-authored-by: armenzg <44410+armenzg@users.noreply.github.com>

jangjodi added a commit that referenced this pull request Jan 18, 2023

feat(codecov): Fetch code coverage for stacktrace (#43116)

3758fa8

Fixes WOR-2550 Co-authored-by: Snigdha Sharma <snigdha.sharma@sentry.io> Co-authored-by: @armenzg <armenzg@sentry.io>

jangjodi added a commit that referenced this pull request Jan 19, 2023

feat(codecov): Fetch code coverage for stacktrace (#43116) (#43407)

60bbc61

Fixes WOR-2550 Co-authored-by: Snigdha Sharma <snigdha.sharma@sentry.io> Co-authored-by: @armenzg <armenzg@sentry.io>

armenzg added this to the Stacktrace Codecov Integration milestone Jan 20, 2023

github-actions bot locked and limited conversation to collaborators Feb 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codecov): Fetch code coverage for stacktrace #43116

feat(codecov): Fetch code coverage for stacktrace #43116

jangjodi commented Jan 11, 2023 •

edited

Loading

snigdhas Jan 11, 2023

snigdhas Jan 11, 2023

snigdhas Jan 11, 2023

jangjodi Jan 11, 2023

snigdhas Jan 11, 2023

jangjodi Jan 11, 2023

snigdhas Jan 11, 2023

jangjodi Jan 11, 2023

snigdhas Jan 12, 2023

jangjodi Jan 12, 2023

armenzg Jan 18, 2023

armenzg Jan 18, 2023

jangjodi Jan 18, 2023

armenzg Jan 18, 2023

armenzg Jan 18, 2023

armenzg Jan 18, 2023

getsentry-bot commented Jan 18, 2023

	repo_name = config["repoName"].split("/")
	owner_username, repo_name = config["repoName"].split("/")

	) -> Iterable[Union[Any, Optional[int]]]:
	) -> Tuple[Sequence[Sequence[int, int]], Optional[int]]:

		CODECOV_TOKEN = options.get("codecov.client-secret")


		class Codecov:

		@@ -0,0 +1,31 @@
		from typing import Optional, Sequence, Tuple

feat(codecov): Fetch code coverage for stacktrace #43116

feat(codecov): Fetch code coverage for stacktrace #43116

Conversation

jangjodi commented Jan 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

getsentry-bot commented Jan 18, 2023

jangjodi commented Jan 11, 2023 •

edited

Loading