feat(instrumentation-knex): Use newer semantic conventions

[deejay1]

Which problem is this PR solving?

• Updates deprecated attributes with current ones and drop deprecated function

Short description of the changes

• Uses newer semantic conventions and replaces deprecated SpanAttributes call with Attributes
• Syncs attributes with PostgreSQL instrumentation

[github-actions]

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

[codecov]

Codecov Report

Attention: Patch coverage is 81.08108% with 7 lines in your changes missing coverage. Please review.

Project coverage is 89.46%. Comparing base (d579630) to head (aef24d6).

Files with missing lines	Patch %	Lines
...emetry-instrumentation-knex/src/instrumentation.ts	72.00%	7 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2671   +/-   ##
=======================================
  Coverage   89.46%   89.46%           
=======================================
  Files         174      176    +2     
  Lines        8322     8355   +33     
  Branches     1592     1601    +9     
=======================================
+ Hits         7445     7475   +30     
- Misses        877      880    +3     

Files with missing lines	Coverage Δ
...lemetry-instrumentation-knex/src/internal-types.ts	100.00% <100.00%> (ø)
.../opentelemetry-instrumentation-knex/src/semconv.ts	100.00% <100.00%> (ø)
...de/opentelemetry-instrumentation-knex/src/utils.ts	89.47% <ø> (ø)
...emetry-instrumentation-knex/src/instrumentation.ts	92.00% <72.00%> (-6.74%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[trentm]

I'll sponsor.

[trentm]

@maryliag I'd appreciate your guidance here. Part of this PR is updating some db.* semconv attribute usage to more recent forms. E.g. changing db.sql.table to db.collection.name (which was part of semconv 1.25.0).

Is that something we should be doing now? Or is that something we should wait on until after the semconv v1.33.0 release because of the pending stabilization of db semconv: https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.32.0 says:

This release is the second release candidate for the Database Semantic Conventions, with db conventions stability planned to be declared in the subsequent release.

Do you know if there will be a db stability migration thing using OTEL_SEMCONV_STABILITY_OPT_IN as was done for http (https://opentelemetry.io/docs/specs/semconv/non-normative/http-migration/)?

[deejay1]

@trentm as far as I recall knex is already behind other modules regarding the attributes, so I don't see much of an issue making these changes opt-in.

[maryliag]

I think it's okay to do them now, because we don't plan on making any other changes to them, at least not in the ones that were updated here. Regarding the opt-in, we do have some guidance on this on semantic convention pages already:

Existing database instrumentations that are using v1.24.0 of this document (or prior):

• SHOULD NOT change the version of the database conventions that they emit by default until the database semantic conventions are marked stable. Conventions include, but are not limited to, attributes, metric and span names, and unit of measure.
• SHOULD introduce an environment variable OTEL_SEMCONV_STABILITY_OPT_IN in the existing major version which is a comma-separated list of values. The list of values includes:
  • database - emit the new, stable database conventions, and stop emitting the old experimental database conventions that the instrumentation emitted previously.
  • database/dup - emit both the old and the stable database conventions, allowing for a seamless transition.
  • The default behavior (in the absence of one of these values) is to continue emitting whatever version of the old experimental database conventions the instrumentation was emitting previously.
  • Note: database/dup has higher precedence than database in case both values are present
• SHOULD maintain (security patching at a minimum) the existing major version for at least six months after it starts emitting both sets of conventions.
• SHOULD drop the environment variable in the next major version.

So, I would say you can make the changes now, but with an opt-in for the new names and still sending old values, then create an issue to remove the old names on a following version, just to make sure we keep track of this and don't forget

[deejay1]

So, I would say you can make the changes now, but with an opt-in for the new names and still sending old values, then create an issue to remove the old names on a following version, just to make sure we keep track of this and don't forget
Ok, I'll adapt this then. Thanks for the feedback.

[deejay1]

@maryliag can you merge it or should I ping someone?

[maryliag]

cc @trentm