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

[aggregator] if sample rate is bad, fix it but still parse tags. #3073

Merged
merged 1 commit into from
Dec 13, 2016

Conversation

truthbk
Copy link
Member

@truthbk truthbk commented Dec 9, 2016

What does this PR do?

We seem to have a bug where we would not parse the tags if a bad rate was reported, we wouldn't even attempt to parse them. This fix will simply correct the bad rate like we were doing earlier without raising an assert that would stop us from grabbing the tags.

Motivation

Behavior was identified by customer.

@truthbk truthbk added the bugfix label Dec 9, 2016
@truthbk truthbk added this to the 5.10.2 milestone Dec 9, 2016
@degemer degemer self-assigned this Dec 12, 2016
Copy link
Member

@degemer degemer left a comment

Choose a reason for hiding this comment

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

Nice! Small nit, LGTM.

sample_rate = 1 # In case it's in a bad state

# in case it's in a bad state
sample_rate = 1 if sample_rate < 0 or sample_rate > 1 else sample_rate
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of moving it where the assert was ? I think it's easier to read, since it's the only case it's used.

@truthbk
Copy link
Member Author

truthbk commented Dec 13, 2016

Failure is unrelated. Merging.

@truthbk truthbk merged commit 6df4554 into master Dec 13, 2016
@truthbk truthbk deleted the jaime/ratetagbug branch December 13, 2016 00:04
degemer added a commit that referenced this pull request Dec 21, 2016
* master: (53 commits)
  [nginx] Update example config
  [service_discovery] Add a Zookeeper service discovery implementation.
  [aggregator] if sample rate is bad, fix it but still parse tags. (#3073)
  [yarn] whitelist authorized application_tags
  Alex poe/update jmx with refresh beans (#3068)
  [config] Fix `_is_affirmative` when passed argument is `None` (#3063)
  Send all configured tags with process checks. (#2976)
  fix flake8 errors
  [flare] ignore whitespace before proxy credentials
  [core] add a switch to disable profiling, but still use developer mode (#2898)
  [tests] allow tests to use the additional_checksd parameter (#3056)
  [service_discovery][jmx] trying to pick-up JMX changes with SD. (#3010)
  [install_script] Make `dd-agent` group of `datadog.conf` (#3036)
  [postgres] Allow disable postgresql.database_size (#3035)
  [core] Fixes IndexError for process lookup (#3043)
  remove warning message leaking password strings (#3053)
  trap psutil.NoSuchProcess exception (#3052)
  Fix grammar and casing in exception text (#3050)
  allow override of kubelet host with KUBERNETES_KUBELET_HOST env var
  [service discovery] properly handle config reload for removed containers
  ...
@masci masci modified the milestones: 5.10.2, 5.12.0 Jan 24, 2017
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.

3 participants