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

Tags support #280

Merged
merged 25 commits into from
Jul 21, 2021
Merged

Tags support #280

merged 25 commits into from
Jul 21, 2021

Conversation

kolesnikovae
Copy link
Collaborator

@kolesnikovae kolesnikovae commented Jul 13, 2021

@kolesnikovae kolesnikovae self-assigned this Jul 13, 2021
@codecov
Copy link

codecov bot commented Jul 13, 2021

Codecov Report

Merging #280 (26b117d) into main (0035a99) will decrease coverage by 4.30%.
The diff coverage is 47.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #280      +/-   ##
==========================================
- Coverage   57.48%   53.19%   -4.29%     
==========================================
  Files          69       96      +27     
  Lines        2634     4242    +1608     
==========================================
+ Hits         1514     2256     +742     
- Misses       1000     1771     +771     
- Partials      120      215      +95     
Impacted Files Coverage Δ
cmd/pyroscope/logging.go 66.67% <0.00%> (-33.33%) ⬇️
cmd/pyroscope/main.go 0.00% <0.00%> (ø)
cmd/pyroscope/main_unix.go 0.00% <0.00%> (ø)
pkg/agent/logger.go 0.00% <0.00%> (ø)
pkg/agent/profiler/profiler.go 0.00% <0.00%> (ø)
pkg/agent/spy/spy.go 5.00% <0.00%> (-4.09%) ⬇️
pkg/agent/target/service.go 0.00% <0.00%> (ø)
pkg/agent/target/service_unix.go 0.00% <0.00%> (ø)
pkg/agent/upstream/direct/direct.go 0.00% <0.00%> (ø)
pkg/build/build.go 7.70% <0.00%> (-7.69%) ⬇️
... and 113 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9d6ab7...26b117d. Read the comment docs.

@kolesnikovae kolesnikovae force-pushed the feature/tags-support branch from 53341fd to e6f14ca Compare July 13, 2021 13:23
@kolesnikovae kolesnikovae force-pushed the feature/tags-support branch from e6f14ca to 9a1a942 Compare July 13, 2021 13:40
@kolesnikovae kolesnikovae marked this pull request as ready for review July 19, 2021 16:25
@kolesnikovae kolesnikovae requested a review from petethepig July 19, 2021 16:25
@github-actions github-actions bot requested a review from Rperry2174 July 19, 2021 16:25
This is necessary to avoid transitive import of logrus package in pkg/agent/profiler. In addition, the key itself is primarily used as a segment key, thus it should reside in the corresponding package.
Copy link
Member

@petethepig petethepig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It all looks great. The only thing is I'm a little concerned about the removal of runtime.GC calls. I'm going to merge this PR, and then do some more tests to see if we need to bring runtime.GC back or not.

@petethepig petethepig merged commit a41b2b7 into main Jul 21, 2021
@petethepig petethepig deleted the feature/tags-support branch July 21, 2021 09:40
korniltsev pushed a commit that referenced this pull request Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants