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

chore: speed up builds #1748

Merged
merged 6 commits into from
Dec 18, 2020
Merged

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Dec 14, 2020

This PR aims to speed up compilations by running a single typescript process from the root of the project. CI is marginally faster, but local development is drastically faster.

Summary

  • Use composite flag in tsconfig to build all packages with a single compile command.
    • Depends on references list in tsconfig. This list is automatically kept up to date using update-ts-references
  • version.ts files are now only generated when the version changes
  • Backwards compatibility compilation checks are now included as part of the regular build so there is no reason for them to have their own GHA

Additional benefits

  • build is a single process now and is much faster than running a tsc process for each package
  • tsc --build --watch will now build and watch the whole repo so a change in api will trigger a rebuild and potentially show errors in tracing or other packages.
    • A watch build in a package will have the same behavior only for that package and packages it depends on directly or indirectly.
  • npm run compile in a package will build that package and its dependencies, using incremental builds to skip things that don't need to be rebuilt.

@codecov
Copy link

codecov bot commented Dec 14, 2020

Codecov Report

Merging #1748 (deaa021) into master (27a8d45) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1748      +/-   ##
==========================================
+ Coverage   92.07%   92.09%   +0.01%     
==========================================
  Files         166      166              
  Lines        5593     5593              
  Branches     1199     1199              
==========================================
+ Hits         5150     5151       +1     
+ Misses        443      442       -1     
Impacted Files Coverage Δ
...emetry-core/src/platform/node/RandomIdGenerator.ts 93.75% <0.00%> (+6.25%) ⬆️

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

What has happened to version.ts ?

I would create a script that will generate automatically something like references.ts that probably should not be a part of the repo but should be generated during compilation or once before watch or tests. Then it will be a matter of requiring this file here. I would like to avoid a time consuming process which is also error prone and will be hard to remember to do it when you add a new package etc. This for sure can be automated.

How will this work for examples. Currently I can add examples/nameOfExample to lerna.json and it will bootstrap correctly so I can debug all easily. How will this work after it ?

@dyladan
Copy link
Member Author

dyladan commented Dec 15, 2020

What has happened to version.ts ?

I'm not done yet, but my goal is to remove the update version on every build which starts a new process and does multiple file read/writes for each of 37 packages. On my machine it's a significant source of overhead. And instead do it as a postversion step or similar. I'm still working on where that will live.

I would create a script that will generate automatically something like references.ts that probably should not be a part of the repo but should be generated during compilation or once before watch or tests. Then it will be a matter of requiring this file here. I would like to avoid a time consuming process which is also error prone and will be hard to remember to do it when you add a new package etc. This for sure can be automated.

The tsconfig.json references are auto-generated by the postinstall script in the root package.json.

How will this work for examples. Currently I can add examples/nameOfExample to lerna.json and it will bootstrap correctly so I can debug all easily. How will this work after it ?

Will work the same way. You just have to make sure the project is built first by running npm run compile.

@dyladan
Copy link
Member Author

dyladan commented Dec 15, 2020

Additional benefits:

  • build is a single process now and is much faster than running a tsc process for each package
  • tsc --build --watch will now build and watch the whole repo so a change in api will trigger a rebuild and potentially show errors in tracing or other packages.
  • npm run compile in a package will build that package and its dependencies, using incremental builds to skip things that don't need to be rebuilt.

- copy protos in postcompile
- separate tsconfig for docs
- add backwards compatibility check to regular build
- bump version.ts as part of version change
Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm,
I think it will be good to add some doc in readme (DEV SETUP for example) explaining

  1. what is being generated automatically
  2. How should we boostrap / install things now (if they are any differences etc.)
  3. If I were to create a new package or add a new node dependency how would that work.
  4. What is the typical dev setup / cycle after this change, how can we run a watch for all etc. to be able to debug stuff .

I hope you can address this prior to merge

@dyladan
Copy link
Member Author

dyladan commented Dec 17, 2020

lgtm,
I think it will be good to add some doc in readme (DEV SETUP for example) explaining

  1. what is being generated automatically
  2. How should we boostrap / install things now (if they are any differences etc.)
  3. If I were to create a new package or add a new node dependency how would that work.
  4. What is the typical dev setup / cycle after this change, how can we run a watch for all etc. to be able to debug stuff .

I hope you can address this prior to merge

@obecny e96e138

@dyladan dyladan merged commit 7e5da41 into open-telemetry:master Dec 18, 2020
@dyladan dyladan deleted the fast-build branch December 18, 2020 14:16
dyladan added a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
dyladan added a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
pichlermarc added a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Apr 17, 2023
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
…ttribute from DynamoDB spans (open-telemetry#1748)

* feat: add configuration to customise dynamodb statement serialization

* fix: format readme

* chore: pass DiagLogger to DynamoDB instrumentation

* chore: omit db statement when serializer is not configured or when it returned undefined
Allow for passing operation to serializer

* chore: run lint:fix

---------

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>
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.

4 participants