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

Gcplog targetmanager #3083

Merged
merged 22 commits into from
Jan 19, 2021
Merged

Gcplog targetmanager #3083

merged 22 commits into from
Jan 19, 2021

Conversation

kavirajk
Copy link
Contributor

@kavirajk kavirajk commented Dec 16, 2020

What this PR does / why we need it:

Add new target manager to promtail that adds support for pulling logs out of pubsub topic.

Special notes for your reviewer:

Still In-progress

Checklist

  • Add scrape config for pubsub target manager
  • Make it work with promtail
  • Log formatter for pubsub target
  • Add stream pull support
  • Add configs to control, bulk read, concurrency
  • Support for exclusion filter (Not applicable here. will be part of terraform script on the cloud config script.)
  • Support for reading messages by ordering key (tried adding ordering key, but didn't work as expected)
  • Documentation added
  • Tests updated
  • PR on deployment_tools with promtail config changes (https://github.com/grafana/deployment_tools/pull/6688)
  • Change GOOGLE_APPLICATION_CREDENTIALS we use to have proper rights to pubsub topics as well.(https://github.com/grafana/deployment_tools/pull/6728)
  • Add basic metrics for pubsub target manager.

@kavirajk
Copy link
Contributor Author

Sorry for too many file changes, its the result of go mod tidy, mostly vendors!

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

Took a quick look at this, left a few comments.

Regarding naming, we definitely should call this something GCP related instead of a generic pubsub target.

pkg/promtail/targets/pubsub/targetmanager.go Outdated Show resolved Hide resolved
pkg/promtail/targets/pubsub/formatter.go Outdated Show resolved Hide resolved
@kavirajk kavirajk changed the title [WIP]: Pubsub targetmanager Gcplog targetmanager Jan 11, 2021
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

slim-bean and others added 14 commits January 18, 2021 14:19
- Create uniq label to create stream without out-of-order timestamp
- extract instance id and timestamp from log entry
- Config changes on pubsub manager for testing
- Rewrite timestamp to avoid out-of-order errors on loki
- Minor config changes to Pubsub Target manager
- Rename Pubsub* entities to Gcplog*
- Rename pubsub package into gcplog
- Add flag `KeepIncomingTimestamp` to toggle between either keeping incoming timestamps or rewriting with current timestamp
- Tests for gcplog target
- Docs and basic metrics to gcplog target
@codecov-io
Copy link

Codecov Report

Merging #3083 (7d0a4c7) into master (40a637e) will decrease coverage by 0.02%.
The diff coverage is 49.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3083      +/-   ##
==========================================
- Coverage   63.33%   63.30%   -0.03%     
==========================================
  Files         191      194       +3     
  Lines       16455    16535      +80     
==========================================
+ Hits        10421    10467      +46     
- Misses       5089     5119      +30     
- Partials      945      949       +4     
Impacted Files Coverage Δ
pkg/distributor/http.go 0.00% <0.00%> (ø)
pkg/promtail/scrapeconfig/scrapeconfig.go 12.00% <ø> (ø)
pkg/promtail/targets/gcplog/targetmanager.go 0.00% <0.00%> (ø)
pkg/promtail/targets/gcplog/formatter.go 61.29% <61.29%> (ø)
pkg/promtail/targets/gcplog/target.go 68.62% <68.62%> (ø)
pkg/promtail/targets/file/filetarget.go 64.82% <100.00%> (+0.24%) ⬆️
pkg/promtail/positions/positions.go 46.80% <0.00%> (-11.71%) ⬇️
... and 1 more

@cyriltovena cyriltovena merged commit 6cc41f9 into grafana:master Jan 19, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 49.54955% with 56 lines in your changes missing coverage. Please review.

Project coverage is 63.30%. Comparing base (40a637e) to head (7d0a4c7).

Files with missing lines Patch % Lines
pkg/promtail/targets/gcplog/targetmanager.go 0.00% 24 Missing ⚠️
pkg/promtail/targets/gcplog/target.go 68.62% 15 Missing and 1 partial ⚠️
pkg/promtail/targets/gcplog/formatter.go 61.29% 6 Missing and 6 partials ⚠️
pkg/distributor/http.go 0.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3083      +/-   ##
==========================================
- Coverage   63.33%   63.30%   -0.03%     
==========================================
  Files         191      194       +3     
  Lines       16455    16535      +80     
==========================================
+ Hits        10421    10467      +46     
- Misses       5089     5119      +30     
- Partials      945      949       +4     
Files with missing lines Coverage Δ
pkg/promtail/scrapeconfig/scrapeconfig.go 12.00% <ø> (ø)
pkg/promtail/targets/file/filetarget.go 64.82% <100.00%> (+0.24%) ⬆️
pkg/distributor/http.go 0.00% <0.00%> (ø)
pkg/promtail/targets/gcplog/formatter.go 61.29% <61.29%> (ø)
pkg/promtail/targets/gcplog/target.go 68.62% <68.62%> (ø)
pkg/promtail/targets/gcplog/targetmanager.go 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants