Remove custom standard libraries dependencies from the contrib main build #16722

bogdandrutu · 2022-12-05T22:43:59Z

Component(s)

exporter/loki

Is your feature request related to a problem? Please describe.

Loki exporter brings custom alternatives of standard libraries like go-kit as requirement in the collector build, see:

// loki needs grafana fork to be used
replace github.com/hpcloud/tail => github.com/grafana/tail v0.0.0-20220426200921-98e8eb28ea4c

// using fork for support LogpullReceived and LogpullFields for loki - see https://github.com/cloudflare/cloudflare-go/pull/743
replace github.com/cloudflare/cloudflare-go => github.com/cyriltovena/cloudflare-go v0.27.1-0.20211118103540-ff77400bcb93

// using fork for custom dialer support for loki - see https://github.com/bradfitz/gomemcache/pull/86
replace github.com/bradfitz/gomemcache => github.com/grafana/gomemcache v0.0.0-20220812141859-1e3ae89e91a7

// fork containing a line-buffered logger which should improve logging performance for loki
replace github.com/go-kit/log => github.com/dannykopping/go-kit-log v0.2.2-0.20221002180827-5591c1641b6b

Describe the solution you'd like

Remove dependency on non standard libraries (e.g. replacement of go-kit with a custom library).

Describe alternatives you've considered

No response

Additional context

If nothing happens in the next few weeks we should remove the loki exporter, since using custom builds of libraries is consider a security threat and should be discouraged.

github-actions · 2022-12-05T22:44:25Z

Pinging code owners for exporter/loki: @gramidt @gouthamve @jpkrohling @kovrus @mar4uk. See Adding Labels via Comments if you do not have permissions to add labels yourself.

jpkrohling · 2022-12-06T14:07:26Z

Are those dependencies ending up on the binary? I understand we depend only on the proto files, none of those other dependencies are in the binary.

bogdandrutu · 2022-12-06T17:23:21Z

Are those dependencies ending up on the binary?

I believe so since I have to add the replace statements on these libraries, I do expect golang compiler/linker to be smart enough to ignore the code that we don't need in the final binary (at least I know that is true in other languages).

mar4uk · 2022-12-06T17:26:06Z

These 4 replace directives were introduced in promtailreceiver (#14632)

We agreed with reviewers that we will get rid of these replaces in the future (as soon as possible) and Juraci created an issue 16453

To remove these replaces right now I need to remove promtailreceiver. Can we postpone this issue till the end of January? I will try to get rid of the replaces till then.

Aneurysm9 · 2022-12-06T19:27:55Z

I recommend that we remove the promtailreceiver until this issue is resolved. It will still be tagged and made available for users who want to include it in a custom distribution, but it sounds like we're not yet ready to include it.

jpkrohling · 2022-12-06T19:29:22Z

The promtail receiver isn't part of the releses repository.

jpkrohling · 2022-12-06T19:48:31Z

I believe so since I have to add the replace statements on these libraries, I do expect golang compiler/linker to be smart enough to ignore the code that we don't need in the final binary (at least I know that is true in other languages).

I just checked the resulting binary for an otelcol-contrib and could find traces only of the last dependency:

$ strings otelcol-contrib | grep hpcloud/tail
$ strings otelcol-contrib | grep grafana/tail
$ strings otelcol-contrib | grep cloudflare/cloudflare-go
$ strings otelcol-contrib | grep cyriltovena/cloudflare-go
$ strings otelcol-contrib | grep bradfitz/gomemcache
$ strings otelcol-contrib | grep grafana/gomemcache
$ strings otelcol-contrib | grep go-kit/log
github.com/go-kit/log
github.com/go-kit/log/level
dep     github.com/go-kit/log   v0.2.1
github.com/go-kit/log.(*jsonLogger).Log
github.com/go-kit/log.merge
github.com/go-kit/log.safeString
github.com/go-kit/log.safeString.func1
github.com/go-kit/log.safeError
github.com/go-kit/log.safeError.func1
github.com/go-kit/log.With
github.com/go-kit/log.newContext
github.com/go-kit/log.containsValuer
github.com/go-kit/log.WithPrefix
github.com/go-kit/log.(*context).Log
github.com/go-kit/log.(*logfmtEncoder).Reset
github.com/go-kit/log.glob..func1
github.com/go-kit/log.logfmtLogger.Log
github.com/go-kit/log.logfmtLogger.Log.func1
github.com/go-kit/log.nopLogger.Log
github.com/go-kit/log.(*syncWriter).Write
github.com/go-kit/log.(*syncWriter).Write.func1
github.com/go-kit/log.(*fdSyncWriter).Write
github.com/go-kit/log.(*fdSyncWriter).Write.func1
github.com/go-kit/log.bindValues
github.com/go-kit/log.TimestampFormat
github.com/go-kit/log.TimestampFormat.func1
github.com/go-kit/log.timeFormat.String
github.com/go-kit/log.timeFormat.MarshalText
github.com/go-kit/log.Caller
github.com/go-kit/log.Caller.func1
github.com/go-kit/log.glob..func2
github.com/go-kit/log.init
github.com/go-kit/log.(*jsonLogger).Write
github.com/go-kit/log.jsonLogger.Write
github.com/go-kit/log.(*logfmtLogger).Log
github.com/go-kit/log.(*nopLogger).Log
github.com/go-kit/log.(*fdSyncWriter).Fd
github.com/go-kit/log.(*fdSyncWriter).Lock
github.com/go-kit/log.(*fdSyncWriter).TryLock
github.com/go-kit/log.(*fdSyncWriter).Unlock
type..eq.github.com/go-kit/log.fdSyncWriter
github.com/go-kit/log.fdSyncWriter.Fd
github.com/go-kit/log.(*syncWriter).Lock
github.com/go-kit/log.(*syncWriter).TryLock
github.com/go-kit/log.(*syncWriter).Unlock
type..eq.github.com/go-kit/log.syncWriter
github.com/go-kit/log.(*logfmtEncoder).EncodeKeyval
github.com/go-kit/log.(*logfmtEncoder).EncodeKeyvals
github.com/go-kit/log.(*logfmtEncoder).EndRecord
github.com/go-kit/log.logfmtEncoder.EncodeKeyval
github.com/go-kit/log.logfmtEncoder.EncodeKeyvals
github.com/go-kit/log.logfmtEncoder.EndRecord
type..eq.github.com/go-kit/log.timeFormat
github.com/go-kit/log.(*timeFormat).MarshalText
github.com/go-kit/log.(*timeFormat).String
github.com/go-kit/log/level.(*logger).Log
github.com/go-kit/log/level.(*levelValue).String
github.com/go-kit/log/level.init
type..eq.github.com/go-kit/log/level.levelValue
type..eq.github.com/go-kit/log/level.logger
github.com/go-kit/log/level.NewFilter
github.com/go-kit/log/level.Error
github.com/go-kit/log/level.ErrorValue
github.com/go-kit/log/level.Info
github.com/go-kit/log/level.InfoValue
github.com/go-kit/log.NewLogfmtLogger
github.com/go-kit/log.NewJSONLogger
github.com/go-kit/log.NewSyncWriter
github.com/go-kit/log/level.AllowWarn
github.com/go-kit/log/level.allowed
github.com/go-kit/log/level.AllowInfo
github.com/go-kit/log/level.AllowDebug
github.com/go-kit/log/level.AllowError
github.com/go-kit/log/level.Warn
github.com/go-kit/log/level.WarnValue
github.com/go-kit/log/level.Debug
github.com/go-kit/log/level.DebugValue
github.com/go-kit/log@v0.2.1/json_logger.go
github.com/go-kit/log@v0.2.1/log.go
github.com/go-kit/log@v0.2.1/value.go
github.com/go-kit/log@v0.2.1/logfmt_logger.go
github.com/go-kit/log@v0.2.1/nop_logger.go
github.com/go-kit/log@v0.2.1/sync.go
github.com/go-kit/log@v0.2.1/stdlib.go
github.com/go-kit/log@v0.2.1/level/level.go
dep     github.com/go-kit/log   v0.2.1
$ strings otelcol-contrib | grep dannykopping/go-kit-log
=>      github.com/dannykopping/go-kit-log      v0.2.2-0.20221002180827-5591c1641b6b    h1:G8g9mAKEj9O3RsU6Hd/ow6lIcHarlcUl5omV6sFKEOU=
=>      github.com/dannykopping/go-kit-log      v0.2.2-0.20221002180827-5591c1641b6b    h1:G8g9mAKEj9O3RsU6Hd/ow6lIcHarlcUl5omV6sFKEOU=

bogdandrutu · 2022-12-06T20:18:33Z

On top of this, fyi github.com/grafana/loki uses go 1.19 which is another reason we may have to remove this dependency.

mar4uk · 2022-12-12T14:43:21Z

There are other dependencies that use Go 1.19. I believe that shouldn't be a reason to remove loki. It is probably better to bump up the collector Go version instead of restricting dependencies using Go 1.19

mar4uk · 2022-12-12T16:57:53Z

@bogdandrutu do you think all these 4 replacements are blockers to include promtail receiver to release? Or the go-kit is the most disturbing one?
Only go-kit is used by the collector components besides promtail receiver.
Other dependencies (github.com/hpcloud/tail, github.com/cloudflare/cloudflare-go, github.com/bradfitz/gomemcache) are used only by promtail receiver

Loki team can remove go-kit replacement by using a fork as the main dependency. That will allow removal replacement from the collector contrib right away

disfluxly · 2022-12-22T15:27:07Z

Hey @mar4uk / @jpkrohling - Any update on this by chance? We noticed that the promtail receiver wasn't in 0.68.0 release either, and we're having issues trying to use the builder to build a binary that includes promtail as well.

mar4uk · 2022-12-22T21:16:34Z

@disfluxly could you please send the config for the builder you use?
Currently promtail receiver is not included to the release. Working on fixing it, but probably it will be in the release not sooner than mid/end of January

disfluxly · 2023-01-09T16:09:34Z

@mar4uk:

dist:
    module: github.com/open-telemetry/opentelemetry-collector # the module name for the new distribution, following Go mod conventions.
receivers:
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/receiver/otlpreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/jaegerreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/fluentforwardreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/influxdbreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awsecscontainermetricsreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/kafkareceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/promtailreceiver"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver"
exporters:
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/exporter/loggingexporter"
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/exporter/otlpexporter"
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/exporter/otlphttpexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/fileexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/jaegerexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/jaegerthrifthttpexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/lokiexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusremotewriteexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/zipkinexporter"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter"
processors:
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/processor/batchprocessor"
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/processor/memorylimiterprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/filterprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/groupbyattrsprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/routingprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/attributesprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourceprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/probabilisticsamplerprocessor"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/processor/tailsamplingprocessor"
extensions:
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/extension/ballastextension"
  - gomod: "go.opentelemetry.io/collector VERSION"
    import: "go.opentelemetry.io/collector/extension/zpagesextension"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/extension/healthcheckextension"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/extension/pprofextension"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/extension/oauth2clientauthextension"
  - gomod: "github.com/open-telemetry/opentelemetry-collector-contrib VERSION"
    import: "github.com/open-telemetry/opentelemetry-collector-contrib/extension/oidcauthextension"

We've tried multiple VERSION, hence why I left it in there. We sed replace it in our build pipeline.

With v0.68.0, this ends up throwing the error:

Error: failed to compile the OpenTelemetry Collector distribution: exit status 2. Output:
# github.com/grafana/loki/pkg/util/log
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:24:22: undefined: log.LineBufferedLogger
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:97:24: undefined: log.NewLineBufferedLogger
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:98:8: undefined: log.WithFlushPeriod
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:99:8: undefined: log.WithPrellocatedBuffer
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:100:8: undefined: log.WithFlushCallback
note: module requires Go 1.19

Compiling with Go 1.19 has the same error, just less the module requires Go 1.19 line:

Error: failed to compile the OpenTelemetry Collector distribution: exit status 2. Output:
# github.com/grafana/loki/pkg/util/log
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:24:22: undefined: log.LineBufferedLogger
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:97:24: undefined: log.NewLineBufferedLogger
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:98:8: undefined: log.WithFlushPeriod
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:99:8: undefined: log.WithPrellocatedBuffer
/Users/disflux/go/pkg/mod/github.com/grafana/loki@v1.6.2-0.20221214135803-c71620ae9437/pkg/util/log/log.go:100:8: undefined: log.WithFlushCallback

Edit: Added go 1.19 compilation error

**What this PR does / why we need it**: The PR cloudflare/cloudflare-go#743 to the upstream repo https://github.com/cloudflare/cloudflare-go was closed with the resolution: > We've spoken to the Grafana Loki team and they'll be building out an integration with Logpush so I don't think this is needed from their side anymore. I [spoke](https://raintank-corp.slack.com/archives/C9T1FLN9K/p1666293494583359) to Loki team and found out that moving to Logpush is not on the shortlist plan right now. I suggest moving to [github.com/grafana/cloudflare-go](https://github.com/grafana/cloudflare-go) fork as the main dependency to get rid of replacement directive. https://github.com/grafana/loki/blob/02e0b3ae89fb5b0dc8849448a1cd2911e0efa1a2/go.mod#L320 Another point is replace directives complicate using loki as a dependency in other packages because replace directives should be moved to the upstream package as well. In general, replace was designed as a temporary solution to test code. It shouldn't be used permanently. **Which issue(s) this PR fixes**: Fixes open-telemetry/opentelemetry-collector-contrib#16722, open-telemetry/opentelemetry-collector-contrib#16453 **Special notes for your reviewer**: **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](../CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [ ] Tests updated - [ ] `CHANGELOG.md` updated - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`

mar4uk · 2023-01-16T15:51:12Z

This issue can be closed. All replace directives mentioned in the description were removed

bogdandrutu added enhancement New feature or request needs triage New item requiring triage exporter/loki Loki Exporter labels Dec 5, 2022

jpkrohling self-assigned this Dec 6, 2022

This was referenced Dec 14, 2022

Move to fork github.com/grafana/tail grafana/loki#7931

Merged

Move to fork github.com/grafana/gomemcache grafana/loki#7936

Merged

[chore] Remove replace directives #17029

Merged

codeboten mentioned this issue Dec 19, 2022

receiver/promtailreceiver not in release v0.67.0 #17082

Closed

This was referenced Jan 13, 2023

Remove loki dependency, copy files from logproto #16822

Merged

Move to fork github.com/grafana/cloudflare-go grafana/loki#8133

Merged

mar4uk mentioned this issue Jan 13, 2023

[chore] remove github.com/cyriltovena/cloudflare-go, github.com/dannykopping/go-kit-log replace directives #17577

Merged

fatsheep9146 closed this as completed Jan 17, 2023

viniciusmuller mentioned this issue Jan 23, 2023

Add promtail receiver to contrib v0.70.0 open-telemetry/opentelemetry-collector-releases#272

Closed

mar4uk mentioned this issue Jan 27, 2023

Add promtail receiver #18072

Merged

mar4uk mentioned this issue Mar 22, 2023

REQUEST: New membership for mar4uk open-telemetry/community#1390

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove custom standard libraries dependencies from the contrib main build #16722

Remove custom standard libraries dependencies from the contrib main build #16722

bogdandrutu commented Dec 5, 2022

github-actions bot commented Dec 5, 2022

jpkrohling commented Dec 6, 2022

bogdandrutu commented Dec 6, 2022

mar4uk commented Dec 6, 2022

Aneurysm9 commented Dec 6, 2022

jpkrohling commented Dec 6, 2022

jpkrohling commented Dec 6, 2022

bogdandrutu commented Dec 6, 2022

mar4uk commented Dec 12, 2022

mar4uk commented Dec 12, 2022

disfluxly commented Dec 22, 2022

mar4uk commented Dec 22, 2022

disfluxly commented Jan 9, 2023 •

edited

Loading

mar4uk commented Jan 16, 2023

Remove custom standard libraries dependencies from the contrib main build #16722

Remove custom standard libraries dependencies from the contrib main build #16722

Comments

bogdandrutu commented Dec 5, 2022

Component(s)

Is your feature request related to a problem? Please describe.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

github-actions bot commented Dec 5, 2022

jpkrohling commented Dec 6, 2022

bogdandrutu commented Dec 6, 2022

mar4uk commented Dec 6, 2022

Aneurysm9 commented Dec 6, 2022

jpkrohling commented Dec 6, 2022

jpkrohling commented Dec 6, 2022

bogdandrutu commented Dec 6, 2022

mar4uk commented Dec 12, 2022

mar4uk commented Dec 12, 2022

disfluxly commented Dec 22, 2022

mar4uk commented Dec 22, 2022

disfluxly commented Jan 9, 2023 • edited Loading

mar4uk commented Jan 16, 2023

disfluxly commented Jan 9, 2023 •

edited

Loading