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

initial switch to circleci #702

Merged
merged 15 commits into from
Mar 26, 2020
Merged

initial switch to circleci #702

merged 15 commits into from
Mar 26, 2020

Conversation

lizthegrey
Copy link
Member

@lizthegrey lizthegrey commented Mar 26, 2020

Description:
Build using CircleCI, which is much faster.
-contrib already is doing this and the .circleci file is modeled after that one.

Link to tracking Issue:
Fixes #550
Fixes #711

Testing:
Ran against CircleCI.

Documentation:
README.md and CONTRIBUTING.md updated.

@lizthegrey
Copy link
Member Author

https://github.com/open-telemetry/opentelemetry-collector/settings/branch_protection_rules/5625387 will need to be changed shortly before merging this pull, to remove the Travis requirement, and to add the Circle requirement.

@lizthegrey
Copy link
Member Author

Looks like the tests will need to be split into multiple chunks, as compiling them with test -race all in one binary exceeds the 4GB limit on CircleCI free for open source.

.circleci/config.yml Outdated Show resolved Hide resolved
@lizthegrey
Copy link
Member Author

OMG it finally passed (but took 10 min).

Let's speed this up by splitting off, first, the make rules that can run independently, and then we'll do the rules that need to be sub-parallelized.

@lizthegrey
Copy link
Member Author

lizthegrey commented Mar 26, 2020

Good enough for now. Some further improvements to be made to make testbed/ produce a list of tests it could run, and to allow running only one at a time (so that parallel circleci containers can run each of the 6 testbed tests), but... followup pull for this seems fine.

go.mod Outdated Show resolved Hide resolved
@lizthegrey
Copy link
Member Author

We'll need open-telemetry/community#319 in order to set our required checks to depend upon whole workflow succeeding rather than an individual job name fyi @bogdandrutu

@lizthegrey
Copy link
Member Author

This should be safe to merge once the CircleCI integration is added to GitHub, and once branch protection settings are changed. I'll address parallelizing the loadtests in a separate pull.

@lizthegrey
Copy link
Member Author

@pjanotti preferences for separate pull to fix the data race, or same pull?

@pjanotti
Copy link
Contributor

@lizthegrey it is fine to do both on the same (I guess that this is more likely to be hit on Circle CI)

name: Code coverage
command: bash <(curl -s https://codecov.io/bash)

publish:
Copy link
Contributor

Choose a reason for hiding this comment

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

@owais FYI

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good. I'll update it in a new PR after this gets merged. Thanks @pjanotti

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

LGTM

@bogdandrutu bogdandrutu merged commit bfff609 into open-telemetry:master Mar 26, 2020
@lizthegrey lizthegrey deleted the lizf.circle branch March 26, 2020 18:08
@lizthegrey
Copy link
Member Author

now we need to adjust branch protection settings, btw

hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…lemetry#702)

* auto add system.type dimension to all sa receiver datapoints

* don't prefix system.type with "smartagent-"

* strip "collectd/", "telegraph/", etc. from system.type
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
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.

Test: TestConcurrentNodeAdds data race Migrate to CircleCI
4 participants