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

refactor: Remove more deprecated telemetry types from core-interfaces #19752

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

Conversation

alexvy86
Copy link
Contributor

Description

Removes the remaining deprecated telemetry-related types from core-interfaces:

  • ITelemetryProperties (replaced with the public ITelemetryBaseProperties)
  • TelemetryEventCategory (moved to @fluidframework/telemetry-utils in the past)
  • TelemetryEventPropertyType (replaced with the equivalent, now-public TelemetryBaseEventPropertyType)
  • ITaggedTelemetryPropertyType (replaced with the public Tagged<TelemetryBaseEventPropertyType>)

Breaking Changes

See description above. 3 types have equivalent replacements already available in the package so consumers would only need to update the names of types being imported. 1 type has its replacement in a different package, so consumers would need to change their import to come from the new package (potentially introducing a new dependency; in practice they probably already have this dependency).

Reviewer Guidance

The review process is outlined on this wiki page.

Particular things where I'd like feedback:

  • ITaggedTelemetryPropertyType was internal but its replacement Tagged<TelemetryBaseEventPropertyType> is public. I think that's correct because consumers need to support tagged properties in telemetry events.

  • TelemetryEventPropertyType was public and its replacement/equivalent TelemetryBaseEventPropertyType was alpha, but with the replacement became public to support all the scenarios where the deprecated type is being used. In particular, ITelemetryBaseProperties requires it to be public, which I think is fine.

@alexvy86 alexvy86 requested review from a team as code owners February 22, 2024 17:27
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: tree area: driver Driver related issues area: loader Loader related issues area: odsp-driver area: runtime Runtime related issues public api change Changes to a public API labels Feb 22, 2024
@alexvy86 alexvy86 requested review from markfields and a team February 22, 2024 17:27
@github-actions github-actions bot added the base: main PRs targeted against main branch label Feb 22, 2024
@alexvy86
Copy link
Contributor Author

/azp run Build - client packages

@alexvy86
Copy link
Contributor Author

/azp run repo-policy-check

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +187 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 514.01 KB 514.05 KB +44 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 247.89 KB 247.91 KB +22 Bytes
loader.js 170.65 KB 170.67 KB +22 Bytes
map.js 46.53 KB 46.54 KB +11 Bytes
matrix.js 148.68 KB 148.69 KB +11 Bytes
odspDriver.js 97.34 KB 97.37 KB +33 Bytes
odspPrefetchSnapshot.js 42.28 KB 42.3 KB +22 Bytes
sharedString.js 167.42 KB 167.43 KB +11 Bytes
sharedTree.js 334 KB 334 KB No change
Total Size 1.87 MB 1.87 MB +187 Bytes

Baseline commit: 42ed6d1

Generated by 🚫 dangerJS against a19ed08

Copy link
Contributor

@Josmithr Josmithr left a comment

Choose a reason for hiding this comment

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

1 documentation suggestion, and I sort of randomly suggested changing some import statements to be type-only - take them or leave them :)

Otherwise, looks good to me!

alexvy86 and others added 2 commits February 22, 2024 18:06
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: tree area: dds Issues related to distributed data structures area: driver Driver related issues area: loader Loader related issues area: odsp-driver area: runtime Runtime related issues base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants