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

opentelemetry-collector: 0.31.0 -> 0.33.0 #135243

Closed

Conversation

r-ryantm
Copy link
Contributor

Automatic update generated by nixpkgs-update tools. This update was made based on information from https://github.com/open-telemetry/opentelemetry-collector-contrib/releases.

meta.description for opentelemetry-collector is: "OpenTelemetry Collector"

meta.homepage for opentelemetry-collector is: "https://github.com/open-telemetry/opentelemetry-collector"

meta.changelog for opentelemetry-collector is: ""

Updates performed
  • Golang update
To inspect upstream changes
Impact
Checks done (click to expand)
  • built on NixOS

  • The tests defined in passthru.tests, if any, passed

  • /nix/store/lfz2l8lyzrj0yqya3gbd0n8md4bf2f1i-opentelemetry-collector-0.33.0/bin/otelcontribcol passed the binary check.

  • 1 of 1 passed binary check by having a zero exit code.

  • 0 of 1 passed binary check by having the new version present in output.

  • found 0.33.0 with grep in /nix/store/lfz2l8lyzrj0yqya3gbd0n8md4bf2f1i-opentelemetry-collector-0.33.0

  • found 0.33.0 in filename of file in /nix/store/lfz2l8lyzrj0yqya3gbd0n8md4bf2f1i-opentelemetry-collector-0.33.0

  • directory tree listing: https://gist.github.com/dad6082140a4cbc888981286aeadd904

  • du listing: https://gist.github.com/1e93f92a6d0f6325cd49715cd6ee55c6


Rebuild report (if merged into master) (click to expand)
4 total rebuild path(s)

1 package rebuild(s)

1 x86_64-linux rebuild(s)
1 i686-linux rebuild(s)
1 x86_64-darwin rebuild(s)
1 aarch64-linux rebuild(s)


First fifty rebuilds by attrpath
opentelemetry-collector
Instructions to test this update (click to expand)

Either download from Cachix:

nix-store -r /nix/store/lfz2l8lyzrj0yqya3gbd0n8md4bf2f1i-opentelemetry-collector-0.33.0 \
  --option binary-caches 'https://cache.nixos.org/ https://nix-community.cachix.org/' \
  --option trusted-public-keys '
  nix-community.cachix.org-1:mB9FSh9qf2dCimDSUo8Zy7bkq5CX+/rkCWyvRCYg3Fs=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The Cachix cache is only trusted for this store-path realization.)
For the Cachix download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.

Or, build yourself:

nix-build -A opentelemetry-collector https://github.com/r-ryantm/nixpkgs/archive/71f3892be7d403bd0c0954d686370e4565e7d1ea.tar.gz

After you've downloaded or built it, look at the files and if there are any, run the binaries:

ls -la /nix/store/lfz2l8lyzrj0yqya3gbd0n8md4bf2f1i-opentelemetry-collector-0.33.0
ls -la /nix/store/lfz2l8lyzrj0yqya3gbd0n8md4bf2f1i-opentelemetry-collector-0.33.0/bin


Pre-merge build results

We have automatically built all packages that will get rebuilt due to
this change.

This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.

Result of nixpkgs-review run on x86_64-linux 1

1 package built:
  • opentelemetry-collector

Maintainer pings

cc @uri-canva @kalbasit for testing.

@uri-canva
Copy link
Contributor

Can't build it on darwin:

# github.com/open-telemetry/opentelemetry-collector-contrib/cmd/otelcontribcol
/nix/store/5psc6givws1ww94hbvr8411mb59nqqy4-go-1.16.7/share/go/pkg/tool/darwin_amd64/link: running clang failed: exit status 1
duplicate symbol '_readdrivestat' in:
    /private/tmp/nix-build-opentelemetry-collector-0.33.0.drv-0/go-link-390201380/000029.o
    /private/tmp/nix-build-opentelemetry-collector-0.33.0.drv-0/go-link-390201380/000032.o
ld: 1 duplicate symbol for architecture x86_64
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
builder for '/nix/store/34057slahhipnfp1y3qyji0512yyhz5i-opentelemetry-collector-0.33.0.drv' failed with exit code 1
error: build of '/nix/store/34057slahhipnfp1y3qyji0512yyhz5i-opentelemetry-collector-0.33.0.drv' failed

Will wait for ofborg: https://github.com/NixOS/nixpkgs/pull/135243/checks?check_run_id=3392948661

Copy link
Contributor

@uri-canva uri-canva left a comment

Choose a reason for hiding this comment

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

ofborg hit the same failure.

@SuperSandro2000
Copy link
Member

It is just failing for darwin. We could ignore that because the error seems strange for me.

@uri-canva
Copy link
Contributor

I don't think we should ignore it, it's a regression, the current version works on darwin.

@SuperSandro2000
Copy link
Member

I don't think we should ignore it, it's a regression, the current version works on darwin.

sure, but do you have a darwin machine and know how to fix it? I don't and darwin is a 2nd class citizen because of those weird breakages and that it is harder to test for people.

@uri-canva
Copy link
Contributor

I think any change is hard to test on hardware you don't own, but we shouldn't merge in changes that break a system just because of that, and ofborg is there to catch unintentional breakages.

One can test changes on hardware they don't have access to using vms. A cursory search for macos vms shows up https://github.com/myspaghetti/macos-virtualbox for example. Can't vouch whether it's any good or not, I do have a darwin machine, so I have only needed to set up linux vms.

@uri-canva
Copy link
Contributor

darwin is a 2nd class citizen

I don't think it is, the set of packages on it is definitely smaller than on linux, but I see it as just as supported as any non-nixos linux.

@uri-canva
Copy link
Contributor

This seems to be related: https://groups.google.com/g/golang-nuts/c/sh4bxSnfY3w.

@mx-psi
Copy link

mx-psi commented Sep 29, 2021

Related upstream issue: open-telemetry/opentelemetry-collector-contrib#5425

You can get around this by building without cgo (passing CGO_ENABLED=0 to the build command should work). Official OpenTelemetry Collector builds are done with CGO_ENABLED=0.

@Artturin Artturin marked this pull request as draft October 5, 2021 12:08
@uri-canva
Copy link
Contributor

Related discussion on whether darwin breakages should block updates and how darwin maintainers might not be able to handle the load of reviewing / addressing darwin specific issues: NixOS/rfcs#112

@uri-canva
Copy link
Contributor

Sorry after more discussion in NixOS/rfcs#112 I've realised I was wrong to block this PR because of darwin compatibility, it is ok to proceed by marking darwin as broken.

@uri-canva
Copy link
Contributor

Preparing a new PR to bump to 0.38.0, will try disabling CGO.

@uri-canva
Copy link
Contributor

New PR up, supersedes this one: #145283.

@uri-canva uri-canva closed this Nov 9, 2021
@r-ryantm r-ryantm deleted the auto-update/opentelemetry-collector branch November 10, 2021 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants