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

[chore] Consolidate genai.request.seed #1683

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gyliu513
Copy link
Member

Fixes #

This is a follow up for https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3081/files#r1884746832

/cc @lmolkova @lzchen

Changes

Please provide a brief description of the changes here.

Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.

Merge requirement checklist

@gyliu513 gyliu513 requested review from a team as code owners December 14, 2024 15:21
@gyliu513 gyliu513 changed the title consolidate genai.request.seed [chore] consolidate genai.request.seed Dec 14, 2024
@gyliu513 gyliu513 changed the title [chore] consolidate genai.request.seed [chore] Consolidate genai.request.seed Dec 14, 2024
@gyliu513
Copy link
Member Author

gyliu513 commented Dec 14, 2024

It is weird, I can see the dead link can actually be accessed https://web.archive.org/web/20180321091318/http://www.onlamp.com/pub/a/linux/2000/11/16/LinuxAdmin.html

@lmolkova
Copy link
Contributor

@gyliu513 thanks for the contribution!

The markdown tables are auto-generated from yaml definitions - please check https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#1-modify-the-yaml-model for more details.

Also, we don't remove attributes, we deprecate them - you can find some examples in the https://github.com/open-telemetry/semantic-conventions/blob/main/model/gen-ai/deprecated/registry-deprecated.yaml

We'll also need a changelog record for it - you can find more details on how to create one in https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#4-changelog.

@gyliu513 gyliu513 force-pushed the sc branch 2 times, most recently from ea39a9b to 86bbc5b Compare December 16, 2024 03:39
Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

Thanks!

Left a few minor comments.

Please also update the https://github.com/open-telemetry/semantic-conventions/blob/main/schema-next.yaml - add the following under version 1.29.0

        # https://github.com/open-telemetry/semantic-conventions/pull/1683
        - rename_attributes:
            attribute_map:
              gen_ai.openai.request.seed: gen_ai.request.seed

.chloggen/1683.yaml Outdated Show resolved Hide resolved
.chloggen/1683.yaml Outdated Show resolved Hide resolved
model/gen-ai/spans.yaml Outdated Show resolved Hide resolved
Copy link
Member Author

@gyliu513 gyliu513 left a comment

Choose a reason for hiding this comment

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

Thanks @lmolkova , all comments are addressed.

.chloggen/1683.yaml Outdated Show resolved Hide resolved
.chloggen/1683.yaml Outdated Show resolved Hide resolved
model/gen-ai/spans.yaml Outdated Show resolved Hide resolved
schema-next.yaml Outdated Show resolved Hide resolved
Copy link

@lzchen lzchen left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs More Approval
Development

Successfully merging this pull request may close these issues.

3 participants