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

Changelog needed but it's missing #350

Closed
awendt opened this issue Feb 19, 2018 · 11 comments
Closed

Changelog needed but it's missing #350

awendt opened this issue Feb 19, 2018 · 11 comments
Assignees
Labels
docs Involves documentation
Milestone

Comments

@awendt
Copy link

awendt commented Feb 19, 2018

Hi,

I just upgraded from 0.9.2 to 0.11.2 in one of our Rails apps and found that Travis attempts to send instrumentation, the log is flooded with:

ERROR -- ddtrace: [ddtrace] (/home/travis/build/foo/bar/vendor/bundle/ruby/2.2.0/gems/ddtrace-0.11.2/lib/ddtrace/transport.rb:100:in `rescue in post') Connection refused - connect(2) for "localhost" port 8126

This seems to be related to #176 but our setup worked with 0.9.2. A comment to #176 mentions #256 but it seems that is not related to the enabled config flag. We're using this config:

Rails.configuration.datadog_trace = {
  enabled: Rails.env.production? && !ENV['SKIP_DATADOG_TRACE'],

  auto_instrument: true,

  # more things here
}

where .travis.yml sets SKIP_DATADOG_TRACE=true.

Now I see that the configuration in README.md changed completely but there is no migration path documented.

It would be nice to have a CHANGES.md or HISTORY.md for this project.

Thanks!

@palazzem palazzem added question General inquiry that may or may not involve changes docs Involves documentation labels Feb 19, 2018
@palazzem
Copy link
Contributor

Hello @awendt ! the current migration plan is available for 0.10 -> 0.11 under "Migration from 0.10.x to 0.11.0" section: https://github.com/DataDog/dd-trace-rb/releases/tag/v0.11.0

Probably we should make it visible somewhere else other than GitHub release notes and at this point having a CHANGES.md (even if a duplicate of GitHub release notes) could make things easier. What do you think?

@awendt
Copy link
Author

awendt commented Feb 19, 2018

@palazzem Thanks for the prompt reply. I didn't think to look there, sorry 😖

Copying the information somewhere else might work but you always have to change it in 2 places if there's an error.

What do you think about a CHANGES.md that merely links to the release notes instead?

@palazzem
Copy link
Contributor

This is fair enough! I'll transform this issue in an action item so that it's something we can address immediately. Thank you very much for your suggestion!

@palazzem palazzem removed the question General inquiry that may or may not involve changes label Feb 19, 2018
@palazzem palazzem added this to the 0.11.3 milestone Feb 19, 2018
@delner
Copy link
Contributor

delner commented Feb 20, 2018

I think this is a good idea, too.

@whithajess
Copy link
Contributor

whithajess commented Mar 6, 2018

@palazzem It would be awesome if dd-trace-rb followed advice on http://guides.rubygems.org/patterns/ around semantic versioning i.e

PATCH 0.0.x level changes for implementation level detail changes, such as small bug fixes
MINOR 0.x.0 level changes for any backwards compatible API changes, such as new functionality/features
MAJOR x.0.0 level changes for backwards incompatible API changes, such as changes that will break existing users code if they update

We use Pessimistic version constraints (~>) and because of this release we actually had breaking changes as we routinely bundle update relying on the fact gems we use follow the above rules and do not break our code (again this is rubygems.org advice).

Luckily I guess doing this caused cascading failures on the majority of our test suites so we found this was a breaking change in this gem quickly.

@delner
Copy link
Contributor

delner commented Mar 6, 2018

@whithajess I think this is a good suggestion, and we've already been taking steps to be compliant with this standard. Our master branch is currently tracking our stable release 0.11.x and is receiving only bugfix changes, while our dev branch 0.12-dev is currently tracking our next beta release 0.12.0.betax, which has new feature implementations.

Moving forward this should make the ~> constraint much more reliable.

@whithajess
Copy link
Contributor

whithajess commented Mar 7, 2018

@delner If 0.12.0 is your next Major release then that will not help as you are operating in the Minor space i.e "backwards compatible". you will need to go to 1.0.0 for the pessimistic version constraint ~> to work properly.

@jeremyolliver
Copy link

Use of -dev or beta releases is nice, but don't address the actual issue. The pessimistic operator ~> can be used in both of these ways:

  • pinning a major X version ~> "0.10" which means any version between >= "0.10.0" AND < 1.0
  • pinning a minor Y version ~> "0.10.2" which means >= "0.10.2" AND < 0.11

0.7 through to 0.10 have mostly been feature releases that didn't break backwards compatibility as far as I know (which is appropriate for bumping Y version) - whereas the 0.11 release did break backwards compatibility and would in retrospect have been worth bumping the major version. It might be worth bumping past 1.0 at some point, and ensuring to increase the major version again if backwards compatibility needs to be broken in the future, to prevent other users being caught out like this. Using this format might mean that regular feature releases look like: 1.1, 1.2, 1.3 (or) 0.12, 0.13, 0.14 - but the important thing is that the backwards incompatible releases break that pattern and increase the next number up (e.g. 0.14 -> 1.0, or 1.3 -> 2.0)

@delner
Copy link
Contributor

delner commented Mar 7, 2018

@whithajess SemVer does specify that minor versions < 1.0 can introduce breaking changes. However, the API has stabilized quite a bit, and out of interest of making updates as stable as possible, and we would like to bump to 1.0 soon to fully embrace the post-1.0 SemVer cycle. But to get there, we might need to release some breaking changes before 1.0.

So in order to manage that, until 1.0, 0.y.0 minor versions might introduce breaking changes, but 0.y.z patch versions (e.g. where only z increments) we will be stable and safe.

As such, I'd recommend using ~> 0.11.0 (not ~> 0.11) to pin the minor version while allowing the patch versions to update as they roll out. This should be production safe, and prevent any incompatible releases from breaking builds through backwards incompatible changes.

Versions after 1.0 will not introduce backwards incompatible changes between minor releases (e.g. 1.0 to 1.1) at which point it should be safe to update the version to ~> 1.0.

@palazzem
Copy link
Contributor

palazzem commented Mar 7, 2018

Thanks @delner for describing our current approach. Anyway @whithajess @jeremyolliver, we want to be very cautious about our breaking changes since we know you're spending time in the instrumentation to have better visibility of your infrastructure (and thanks for trusting us!). In the case you've described, we had to introduce the big change 0.10->0.11 to update how we were initializing our integrations. It wasn't just syntactic sugar, but it was required to fix core problems in our tracing library.

Because your feedback is very important, I can reassure you that the 1.0 is at the door and after tweaking some defaults, you can safely upgrade our gem between minor releases. Also our deprecation policy (via log.warn at init time) will help you with any migration between (for instance) 1.5->2.0, so that before introducing a breaking change you'll have time to do the proper change (with a good description of how you can do it).

Thank you very much for the feedbacks!

@delner
Copy link
Contributor

delner commented Mar 12, 2018

We've released a CHANGELOG.md with our latest 0.11.3 release. See #363. Closing this issue.

@delner delner closed this as completed Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Involves documentation
Projects
None yet
Development

No branches or pull requests

5 participants