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

[mdatagen] Add deprecation date and migration note fields for deprecated components #12464

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Feb 24, 2025

Description

This PR adds deprecation date and migration note fields for deprecated components as described at #12359.

Example metadata file:

status:
  class: receiver
  stability:
    development: [logs]
    beta: [traces]
    stable: [metrics]
    deprecated: [profiles]
  deprecation:
    profiles:
      migration: "no migration needed"
      date: "2025-02-05"

Example README.md:

Status
Stability deprecated: profiles
development: logs
beta: traces
stable: metrics
Deprecation of profiles Date: 2025-02-05
Migration Note: no migration needed

I'd appreciate any suggestions if there is a better way to represent this information in the markdown table.

Link to tracking issue

Fixes #12359

Testing

Added

Documentation

Added

/cc @atoulme who filed the feature request issue originally.

@ChrsMark ChrsMark requested review from dmitryax and a team as code owners February 24, 2025 16:00
@ChrsMark ChrsMark force-pushed the mdatagen_deprecation_fields branch from fe6505a to e3b46d3 Compare February 24, 2025 16:01
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.11%. Comparing base (0d81535) to head (ab4f372).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #12464   +/-   ##
=======================================
  Coverage   92.11%   92.11%           
=======================================
  Files         467      467           
  Lines       25252    25276   +24     
=======================================
+ Hits        23260    23284   +24     
  Misses       1590     1590           
  Partials      402      402           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

this PR covers well what I was asking for. LGTM

@ChrsMark ChrsMark force-pushed the mdatagen_deprecation_fields branch 3 times, most recently from 1043c28 to 4181e95 Compare February 24, 2025 19:43
@atoulme
Copy link
Contributor

atoulme commented Feb 24, 2025

I recommend we start adopting the additional yaml in contrib and make contrib tests pass, then merge this PR. We can do that with one PR per deprecated component:

@ChrsMark ChrsMark force-pushed the mdatagen_deprecation_fields branch from 4181e95 to ab22bd5 Compare February 25, 2025 12:05
@ChrsMark
Copy link
Member Author

Does this mean we should create and merge PRs like open-telemetry/opentelemetry-collector-contrib#38144? My concern is that contrib CI will be broken in that case but it looks like chicken-egg problem 🤔 .

@ChrsMark ChrsMark force-pushed the mdatagen_deprecation_fields branch 2 times, most recently from 9299b25 to 57e163e Compare February 25, 2025 15:15
@atoulme
Copy link
Contributor

atoulme commented Feb 25, 2025

#12484 can remove this problem, please take a look?

@dmitryax
Copy link
Member

#12484 can remove this problem, please take a look?

I'm not sure why would we allow random fields going forward. I would suggest the following:

  1. update the mdatagen schema first, but don't enforce it
  2. update usages in contrib
  3. enforce it

@dmitryax
Copy link
Member

Or we can just add Skip Contrib Tests until contrib is updated. PRs there can be ready in the "failing" state until this is merged

@ChrsMark
Copy link
Member Author

#12484 can remove this problem, please take a look?

I'm not sure why would we allow random fields going forward. I would suggest the following:

  1. update the mdatagen schema first, but don't enforce it
  2. update usages in contrib
  3. enforce it

I like this approach. I filed #12498 to update the schema without enforcing it.

@atoulme
Copy link
Contributor

atoulme commented Feb 26, 2025

@ChrsMark please rebase this PR to fix conflicts, thanks!

@ChrsMark
Copy link
Member Author

@ChrsMark please rebase this PR to fix conflicts, thanks!

Sure, will need to rebase anyways once/if #12498 gets merged.

github-merge-queue bot pushed a commit that referenced this pull request Feb 26, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

First PR to update metadata schema without enforcing it as suggested at
#12464 (comment).

<!-- Issue number if applicable -->
#### Link to tracking issue
Related to
#12359

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Signed-off-by: ChrsMark <[email protected]>
@ChrsMark ChrsMark force-pushed the mdatagen_deprecation_fields branch from 57e163e to 4ed4127 Compare February 27, 2025 09:48
@ChrsMark ChrsMark force-pushed the mdatagen_deprecation_fields branch from 4ed4127 to ab4f372 Compare February 27, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mdatagen] add deprecation start date and migration instructions
4 participants