Skip to content

Latest commit

 

History

History
246 lines (168 loc) · 10.6 KB

contributing.md

File metadata and controls

246 lines (168 loc) · 10.6 KB
Raw

Contributing to Datadog Agent

First of all, thanks for contributing!

This document provides some basic guidelines for contributing to this repository. To propose improvements, feel free to submit a PR.

Submitting issues

  • If you think you've found an issue, please search the [Agent Troubleshooting][agent-troubleshooting] section to see if it's known.
  • If you’re still unsure about the issue, you may reach out to the Datadog support team with a flare from your Agent.
  • Finally, you can open a Github issue.

Pull Requests

Have you fixed a bug or written a new check and want to share it? Many thanks!

In order to ease/speed up our review, here are some items you can check/improve when submitting your PR:

=== "Contributor Checklist" - [X] Have a proper commit history (we advise you to rebase if needed) with clear commit messages.

- [X] Write tests for the code you wrote.

- [X] Preferably make sure that all tests pass locally.

- [X] Summarize your PR with an explanatory title and a message describing your changes, cross-referencing any related bugs/PRs.

- [X] Use [Reno](#reno) to create a release note.

- [X] Open your PR against the `main` branch.

- [X] Provide adequate QA/testing plan information.

=== "Reviewer Checklist" - [X] The added code comes with tests.

- [X] The CI is green, all tests are passing (required or not).

- [X] All applicable labels are set on the PR (see [PR labels list](#pr-labels)).

- [X] If applicable, the [config template](https://github.com/DataDog/datadog-agent/blob/main/pkg/config/config_template.yaml) has been updated.

!!! note Adding GitHub labels is only possible for contributors with write access.

Your pull request must pass all CI tests before we will merge it. If you're seeing an error and don't think it's your fault, it may not be! Join us on Slack or send us an email, and together we'll get it sorted out.

Keep it small, focused

Avoid changing too many things at once. For instance if you're fixing the NTP check and at the same time shipping a dogstatsd improvement, it makes reviewing harder and the time-to-release longer.

Commit Messages

Please don't be this person: git commit -m "Fixed stuff". Take a moment to write meaningful commit messages.

The commit message should describe the reason for the change and give extra details that will allow someone later on to understand in 5 seconds the thing you've been working on for a day.

This includes editing the commit message generated by GitHub from:

Including new features

* Fix linter
* WIP
* Add test for x86
* Fix licenses
* Cleanup headers

to:

Including new features

This feature does this and that. Some tests are excluded on x86 because of ...

If your commit is only shipping documentation changes or example files, and is a complete no-op for the test suite, please add [skip ci] in the commit message body to skip the build and give that slot to someone else who does need it.

Pull request workflow

The goals ordered by priority are:

  • Make PR reviews (both initial and follow-up reviews) easy for reviewers using GitHub
  • On the main branch, have a meaningful commit history that allows understanding (even years later) what each commit does, and why.

You must open the PR when the code is reviewable or you must set the PR as draft if you want to share code before it's ready for actual reviews.

Before the first PR review

Before the first PR review, meaningful commits are best: logically-encapsulated commits help the reviews go quicker and make the job for the reviewer easier. Conflicts with main can be resolved with a git rebase origin/main and a force push if it makes future review(s) easier.

After the first review

After the first review, to make follow-up reviews easier:

  • Avoid force pushes: rewriting the history that was already reviewed makes follow-up reviews painful as GitHub loses track of each comment. Instead, address reviews with additional commits on the PR branch.
  • Resolve merge conflicts with main using git merge origin/main

How to merge to main

Once reviews are complete, the merge to main should be done with either:

  • the squash-merge option, to keep the history of main clean (even though some context/details are lost in the squash). The commit message for this squash should always be edited to concisely describe the commit without extraneous “address review comments” text.
  • the “rebase-merge” option, after manually rewriting the PR’s commit history and force-pushing to the branch. When using this option, the branch must have a clean history.

Reno

We use Reno to create our CHANGELOG. Reno is a pretty simple tool.

Each PR should include a releasenotes file created with reno, unless the PR doesn't have any impact on the behavior of the Agent and therefore shouldn't be mentioned in the CHANGELOG (examples: repository documentation updates, changes in code comments). PRs that don't require a release note file will be labeled changelog/no-changelog by maintainers.

To install reno: pip install reno

Ultra quick Reno HOWTO:

$> reno new <topic-of-my-pr> --edit
[...]
# Remove unused sections and fill the relevant ones.
# Reno will create a new file in releasenotes/notes.
#
# Each section from every release note are combined when the CHANGELOG.rst is
# rendered. So the text needs to be worded so that it does not depend on any
# information only available in another section. This may mean repeating some
# details, but each section must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
[...]

Then just add and commit the new releasenote (located in releasenotes/notes/) with your PR. If the change is on the trace-agent (folders cmd/trace-agent or pkg/trace) please prefix the release note with "APM :" and the argument with "apm-".

Reno sections

The main thing to keep in mind is that the CHANGELOG is written for the agent's users and not its developers.

  • features: describe shortly what your feature does.

    example:

    features:
  - |
    Introducing the Datadog Process Agent for Windows.

  • enhancements: describe enhancements here: new behavior that are too small to be considered a new feature.

    example:

    enhancements:
  - |
    Windows: Add PDH data to flare.

  • issues: describe known issues or limitation of the agent.

    example:

    issues:
  - |
    Kubernetes 1.3 & OpenShift 3.3 are currently not fully supported: docker
    and kubelet integrations work OK, but apiserver communication (event
    collection, `kube_service` tagging) is not implemented

  • upgrade: List actions to take or limitations that could arise upon upgrading the Agent. Notes here must include steps that users can follow to 1. know if they're affected and 2. handle the change gracefully on their end.

    example:

    upgrade:
  - |
    If you run a Nomad agent older than 0.6.0, the `nomad_group`
    tag will be absent until you upgrade your orchestrator.

  • deprecations: List deprecation notes here.

    example:

    deprecations:
- |
  Changed the attribute name to enable log collection from YAML configuration
  file from "log_enabled" to "logs_enabled", "log_enabled" is still
  supported.

  • security: List security fixes, issues, warning or related topics here.

    example:

    security:
  - |
    The /agent/check-config endpoint has been patched to enforce
    authentication of the caller via a bearer session token.

  • fixes: List the fixes done in your PR here. Remember to be clear and give a minimum of context so people reading the CHANGELOG understand what the fix is about.

    example:

    fixes:
  - |
    Fix EC2 tags collection when multiple marketplaces are set.

  • other: Add here every other information you want in the CHANGELOG that don't feat in any other section. This section should rarely be used.

    example:

    other:
  - |
    Only enable the ``resources`` metadata collector on Linux by default, to match
    Agent 5's behavior.

PR labels

For internal PRs (from people in the Datadog organization), you have few extra labels that can be use:

  • community/help-wanted: for community PRs where help is needed to finish it.

  • community: for community PRs.

  • changelog/no-changelog: for PRs that don't require a reno releasenote (useful for PRs only changing documentation or tests).

  • qa/done or qa/no-code-change: used to skip the QA week:

    • qa/done label is recommended in case of code changes and manual / automated QA done before merge.
    • qa/no-code-change is recommended if there's no code changes in the Agent binary code.

    !!! note "Important" Use qa/no-code-change if your PR only changes tests or a module/package that does not end up in the Agent build. All of the following do not require QA:

      - Changing the CI configuration without impacting the Agent packaging.
  - Changing the documentation.
  - Changing the developer tooling.

  • major_change: to flag the PR as a major change impacting many/all teams working on the agent and will require deeper QA (example: when we change the Python version shipped in the agent).

  • need-change/operator, need-change/helm: indicate that the configuration needs to be modified in the operator / helm chart as well.

  • k8s/<min-version>: indicate the lowest Kubernetes version compatible with the PR's feature.

  • backport/<branch-name>: Add this label to automatically create a PR against the <branch-name> branch with your backported changes. The backport PR creation is triggered:

    • When a PR with the label is merged
    • When an already-merged PR gets the label

    If there is a conflict, the bot prompts you with a list of instructions to follow (example) to manually backport your PR.

Integrations

Also called checks, all officially supported Agent integrations live in the integrations-core repo. Please look there to submit related issues, PRs, or review the latest changes. For new integrations, please open a pull request in the integrations-extras repo.