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.

Sign up for GitHub

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

Jump to bottom

Deprecate use_latest_spec option #14446

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Deprecate use_latest_spec option #14446

merged 1 commit into from
Apr 25, 2023

Conversation

alopezz
Copy link
Contributor

@alopezz alopezz commented Apr 24, 2023

What does this PR do?

Hides use_latest_spec option, as it's now left as a last resort in case the server exposing the metrics fails to set the appropiate header.

Motivation

Follow-up.

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@alopezz alopezz requested review from a team as code owners April 24, 2023 12:46
@alopezz alopezz mentioned this pull request Apr 24, 2023
Merged
5 tasks
@alopezz Graphite App
Copy link
Contributor Author

alopezz commented Apr 24, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@ghost ghost added the integration/vault label Apr 24, 2023
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

maycmlee
maycmlee reviewed Apr 24, 2023
View reviewed changes
Copy link
Contributor

@maycmlee maycmlee left a comment

Choose a reason for hiding this comment

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

Nothing for docs to review

hithwen
hithwen previously approved these changes Apr 24, 2023
View reviewed changes
FlorentClarret
FlorentClarret previously approved these changes Apr 24, 2023
View reviewed changes
Copy link
Member

@FlorentClarret FlorentClarret left a comment

Choose a reason for hiding this comment

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

nit - this should be changelog/fixed as these files are shipped with the agent

@alopezz
Copy link
Contributor Author

alopezz commented Apr 24, 2023

nit - this should be changelog/fixed as these files are shipped with the agent

I was on the fence with this one, as it technically doesn't change behaviour. But I'm also deferring this decision to what I mention in #14445's "Additional notes".

@alopezz alopezz force-pushed the alopez/openmetrics/dynamic-parser-selection branch from 70d3df2 to 6121ec6 Compare April 24, 2023 16:23
@alopezz alopezz force-pushed the alopez/openmetrics/deprecate-use_latest_spec branch from 3a46a08 to 5384d0a Compare April 24, 2023 16:23
@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@alopezz alopezz force-pushed the alopez/openmetrics/dynamic-parser-selection branch from 6121ec6 to 1abfce5 Compare April 25, 2023 08:01
Base automatically changed from alopez/openmetrics/dynamic-parser-selection to master April 25, 2023 10:20
@alopezz alopezz dismissed stale reviews from FlorentClarret and hithwen via 5384d0a April 25, 2023 10:20
@codecov
Copy link

codecov bot commented Apr 25, 2023 •
edited
Loading

Codecov Report

Merging #14446 (b6abb6a) into master (7d8dc6d) will increase coverage by 0.00%.
The diff coverage is n/a.

Flag Coverage Δ
datadog_checks_dev 82.71% <ø> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@alopezz
Deprecate use_latest_spec option
b6abb6a 
Hiding it because it should be no longer needed normally, now that we
rely on the content type header to determine the incoming format.
@alopezz alopezz force-pushed the alopez/openmetrics/deprecate-use_latest_spec branch from 5384d0a to b6abb6a Compare April 25, 2023 11:23
FlorentClarret
FlorentClarret approved these changes Apr 25, 2023
View reviewed changes
Copy link
Member

@FlorentClarret FlorentClarret left a comment

Choose a reason for hiding this comment

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

🚀

@alopezz alopezz merged commit 74713c0 into master Apr 25, 2023
@alopezz alopezz deleted the alopez/openmetrics/deprecate-use_latest_spec branch April 25, 2023 13:02
@alopezz alopezz mentioned this pull request Apr 25, 2023
Closed
5 tasks
@FlorentClarret FlorentClarret mentioned this pull request Apr 28, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maycmlee maycmlee maycmlee left review comments

@FlorentClarret FlorentClarret FlorentClarret approved these changes

@hithwen hithwen hithwen left review comments

Assignees
No one assigned
Labels
dev_package dev/tooling integration/aerospike integration/amazon_msk integration/arangodb integration/argocd integration/avi_vantage integration/boundary integration/calico integration/cert_manager integration/cilium integration/cockroachdb integration/coredns integration/envoy integration/gitlab integration/haproxy integration/impala integration/istio integration/kong integration/linkerd integration/openmetrics integration/pulsar integration/rabbitmq integration/teamcity integration/temporal integration/vault
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alopezz @hithwen @FlorentClarret @maycmlee