Added build step for generating Windows MSI #1153

james-bebbington · 2020-06-22T06:19:33Z

Link to tracking Issue:
#1125

Description:
Added commands & build steps to generate an MSI.

Running .\scripts\make.ps1 New-MSI -Config "./path/to/conf.yaml" will create an MSI packaged with the specified config file. If the -Config argument is not provided, the default config file in examples/otel-local-config.yaml will be used.
An MSI with the default config will be published as a release artifact.

Users are able to override the config when installing the MSI by passing in a different config file path as an argument, i.e.

msiexec /i opentelemetry-collector.msi CONFIG=path/to/conf.yaml

If the user specifies a custom config file, and that file can't be found, they will get a generic popup error. The MSI logs will report "CustomAction CopyConfig returned actual error code 4" which is a pretty mediocre error message, but at least points to the error.

(once this gets merged, I'll create a similar PR in Contrib)

codecov · 2020-06-22T07:14:37Z

Codecov Report

Merging #1153 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1153      +/-   ##
==========================================
- Coverage   88.08%   88.06%   -0.03%     
==========================================
  Files         203      203              
  Lines       14658    14658              
==========================================
- Hits        12912    12908       -4     
- Misses       1309     1311       +2     
- Partials      437      439       +2

Impacted Files	Coverage Δ
translator/internaldata/resource_to_oc.go	`83.72% <0.00%> (-4.66%)`	⬇️

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 12b3d9c...3ad05c4. Read the comment docs.

james-bebbington

I need a bit of guidance with this PR. Please see the questions below:

When should this step run? Shall we set this up similarly to the publish-dev & publish-stable steps? (can't be combined into the same steps as will need to run under a diff executor unfortunately)
Where should we store the generated MSI? Afaik, there isn't really any standard package manager for storing MSIs. Is storing it as a release artifact enough? That would mean we only run this on publish-stable. Should we run a step to create the MSI on merge anyway, so we find out sooner if this somehow gets broken?
Also see question below re Makefile

@jrcamp @owais

james-bebbington · 2020-06-26T02:14:09Z

.circleci/config.yml

+          name: Install Wix Toolset
+          command: |
+            choco install wixtoolset
+            setx /m PATH "%PATH%;C:\Program Files (x86)\WiX Toolset v3.11\bin"


Annoyingly we have to do this because the choco package doesn't take care of it for us: https://chocolatey.org/packages/wixtoolset#comment-3471097677

Makefile

jrcamp · 2020-06-26T19:34:18Z

I need a bit of guidance with this PR. Please see the questions below:

When should this step run? Shall we set this up similarly to the publish-dev & publish-stable steps? (can't be combined into the same steps as will need to run under a diff executor unfortunately)

Where should we store the generated MSI? Afaik, there isn't really any standard package manager for storing MSIs. Is storing it as a release artifact enough? That would mean we only run this on publish-stable. Should we run a step to create the MSI on merge anyway, so we find out sooner if this somehow gets broken?

Also see question below re Makefile

Do workspaces persist across executor types? I forget. If so then maybe you can add windows-msi to requires of publish-stable? And in the job store the msi using store_artifacts?

That would mean we only run this on publish-stable. Should we run a step to create the MSI on merge anyway, so we find out sooner if this somehow gets broken?

Not sure what you mean by this. It was run as part of the circleci job in this PR for instance even though it's not part of publish-stable right?

jrcamp · 2020-06-26T19:37:25Z

btw would be nice to get into chocolatey repo (@jchengsfx did it for smart agent recently)

jchengsfx

@james-bebbington Let's rename the scripts/msi/ directory to packaging/msi/, move make.ps1 to packaging/msi/, and update the paths in the files accordingly.

jrcamp · 2020-06-29T18:14:11Z

@james-bebbington I think it's fine to build msi on every build. Like you say we don't want to find out it's broken after we make a release. It should happen in parallel to other things like running tests so I don't think it should make the build much longer.

james-bebbington · 2020-06-30T04:34:50Z

@james-bebbington Let's rename the scripts/msi/ directory to packaging/msi/, move make.ps1 to packaging/msi/, and update the paths in the files accordingly.

Cool - all done

btw would be nice to get into chocolatey repo (@jchengsfx did it for smart agent recently)

I'll see how I go for time. Would be nice to add this, but we likely won't need it as we have another package manager for Windows on GCP.

jchengsfx · 2020-06-30T14:35:50Z

packaging/msi/make.ps1

+function New-MSI(
+    [string]$Config="./examples/otel-local-config.yaml"
+) {
+    $Version = If($env:VERSION) { $env:VERSION } else { "0.0.1" }


We should also set VERSION to the CIRCLE_TAG env var (if it exists and matches \d+\.\d+\.\d+, without the v) for release builds.

Done - since we're referring to a CircleCI specific env variable, I moved this code into the CircleCI config file.

Minor note: MSI Product Version only supports the regex format you specified (e.g. 0.0.1). The tag filter we use to determine when to publish releases allows additional characters at the end (e.g. v0.0.1-alpha). If we see a tag like that I've opted to just ignore the extra chars.

james-bebbington · 2020-07-03T00:20:02Z

.circleci/config.yml

@@ -73,7 +73,7 @@ workflows:
              only: /^v([0-9])+.([0-9])+.([0-9])+.*/
      - cross-compile:
          requires:
-            - build
+            - setup-and-lint


@owais

I think this PR is ready to be merged, however, should I make the same change here as you made in https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/384/files and replace build with cross-compile? (I had changed this dependency to speed up the overall build time since this PR adds another step that's dependent on cross-compile)

james-bebbington · 2020-07-07T05:57:22Z

I'm very keen to merge this one soon if anyone is able to take another look

…o the spec (open-telemetry#1153) * Rename ParentOrElse to ParentBased and enhance it according to the spec - Renaming of type and sampler function - Enhancing ParentBased with sampler options - Set default samplers for each applicable parent-based case - Adjust ShouldSample(...) func accordingly * Adjust existing tests for ParentBased and add new ones - add tests for ParentBased sampler options and description - renaming in trace_test.go * Update CHANGELOG.md * PR feedback - More clearer naming of structs; add comments where missing - Adhere to the configuration style guide * PR feedback - punctuation

james-bebbington requested review from bogdandrutu, dmitryax, flands, owais, pjanotti, rghetia, songy23 and tigrannajaryan as code owners June 22, 2020 06:19

james-bebbington marked this pull request as draft June 22, 2020 06:29

james-bebbington force-pushed the windows-msi branch from 692180d to c856f6f Compare June 22, 2020 07:08

james-bebbington force-pushed the windows-msi branch 8 times, most recently from a451187 to 5e78a80 Compare June 22, 2020 11:23

james-bebbington mentioned this pull request Jun 25, 2020

Enable Collector to be run as a Windows service #1120

Merged

james-bebbington force-pushed the windows-msi branch 2 times, most recently from a2b5323 to 7f09b7c Compare June 26, 2020 02:06

james-bebbington commented Jun 26, 2020

View reviewed changes

james-bebbington changed the title ~~[WIP] Added build step for generating Windows MSI~~ Added build step for generating Windows MSI Jun 26, 2020

james-bebbington marked this pull request as ready for review June 26, 2020 02:40

james-bebbington force-pushed the windows-msi branch from 7f09b7c to 67fd4b2 Compare June 26, 2020 02:48

tigrannajaryan added this to In progress in Collector via automation Jun 26, 2020

james-bebbington force-pushed the windows-msi branch from 67fd4b2 to a6bbeef Compare June 29, 2020 06:49

jchengsfx reviewed Jun 29, 2020

View reviewed changes

tigrannajaryan assigned jchengsfx Jun 29, 2020

james-bebbington force-pushed the windows-msi branch 3 times, most recently from 4eb6cfb to 6a1d522 Compare June 30, 2020 04:31

james-bebbington force-pushed the windows-msi branch 4 times, most recently from cfb43a7 to 9c19105 Compare June 30, 2020 07:44

Collector automation moved this from In progress to Review in progress Jun 30, 2020

jchengsfx suggested changes Jun 30, 2020

View reviewed changes

james-bebbington force-pushed the windows-msi branch 4 times, most recently from a900b7f to 28f3ec8 Compare July 1, 2020 05:39

Added wix file and build step to create msi

3ad05c4

james-bebbington force-pushed the windows-msi branch from 28f3ec8 to 3ad05c4 Compare July 1, 2020 05:52

james-bebbington commented Jul 3, 2020

View reviewed changes

jchengsfx approved these changes Jul 7, 2020

View reviewed changes

bogdandrutu approved these changes Jul 7, 2020

View reviewed changes

Collector automation moved this from Review in progress to Reviewer approved Jul 7, 2020

bogdandrutu merged commit db02bc3 into open-telemetry:master Jul 7, 2020

Collector automation moved this from Reviewer approved to Done Jul 7, 2020

james-bebbington mentioned this pull request Jul 8, 2020

Allow Collector to be run as a Windows Service & add MSI build step open-telemetry/opentelemetry-collector-contrib#408

Merged

wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020

Added wix file and build step to create msi (open-telemetry#1153)

ae75bc5

jchengsfx mentioned this pull request Jul 22, 2020

REQUEST: New membership for jchengsfx open-telemetry/community#422

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added build step for generating Windows MSI #1153

Added build step for generating Windows MSI #1153

james-bebbington commented Jun 22, 2020 •

edited

codecov bot commented Jun 22, 2020 •

edited

james-bebbington left a comment •

edited

james-bebbington Jun 26, 2020

jrcamp commented Jun 26, 2020

jrcamp commented Jun 26, 2020

jchengsfx left a comment

jrcamp commented Jun 29, 2020

james-bebbington commented Jun 30, 2020

jchengsfx Jun 30, 2020 •

edited

james-bebbington Jul 1, 2020

james-bebbington Jul 3, 2020 •

edited

james-bebbington commented Jul 7, 2020

Added build step for generating Windows MSI #1153

Added build step for generating Windows MSI #1153

Conversation

james-bebbington commented Jun 22, 2020 • edited

codecov bot commented Jun 22, 2020 • edited

Codecov Report

james-bebbington left a comment • edited

Choose a reason for hiding this comment

james-bebbington Jun 26, 2020

Choose a reason for hiding this comment

jrcamp commented Jun 26, 2020

jrcamp commented Jun 26, 2020

jchengsfx left a comment

Choose a reason for hiding this comment

jrcamp commented Jun 29, 2020

james-bebbington commented Jun 30, 2020

jchengsfx Jun 30, 2020 • edited

Choose a reason for hiding this comment

james-bebbington Jul 1, 2020

Choose a reason for hiding this comment

james-bebbington Jul 3, 2020 • edited

Choose a reason for hiding this comment

james-bebbington commented Jul 7, 2020

james-bebbington commented Jun 22, 2020 •

edited

codecov bot commented Jun 22, 2020 •

edited

james-bebbington left a comment •

edited

jchengsfx Jun 30, 2020 •

edited

james-bebbington Jul 3, 2020 •

edited