Skip to content

[internal/k8sinventory] Extract checkpoint package and update Observer interface - #48765

Merged
ChrsMark merged 6 commits into
open-telemetry:mainfrom
kangyili:k8sobject/component
Jul 13, 2026
Merged

[internal/k8sinventory] Extract checkpoint package and update Observer interface#48765
ChrsMark merged 6 commits into
open-telemetry:mainfrom
kangyili:k8sobject/component

Conversation

@kangyili

@kangyili kangyili commented May 29, 2026

Copy link
Copy Markdown
Contributor

Description

Preparatory refactor split from #48663 to keep that PR reviewable.

This PR introduces no behavioral changes, it only refactors code required for the informer adoption on k8s_objects receiver.

  1. Extract checkpoint package: watch/checkpointer.go was private to the watch package but the incoming informer-based observer also needs it. Promoted to a shared internal/k8sinventory/checkpoint package — behavior unchanged.
  2. Add 2 new methodsLoad() and AlreadySeen() to the Checkpointer which will be used by the informer changes.
  3. Observer.Start returns error: the informer does synchronous initialization before returning (cache sync), which can time out or fail. Returning (chan struct{}, error) lets the receiver fail fast with a clear signal rather than silently continuing with a broken observer. The old observers had no synchronous initialization, so this interface change was necessary to surface failures from the new one.

Link to tracking issue

#43602

Comment thread internal/k8sinventory/checkpoint/checkpointer.go Outdated

@ChrsMark ChrsMark left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM overall and apologies for the delay. Left 2 comments about some error handling but other than this it looks good to me.

@TylerHelmuth @dmitryax PTAL

Comment thread internal/k8sinventory/checkpoint/checkpointer.go Outdated
Comment thread internal/k8sinventory/checkpoint/checkpointer.go Outdated

@ChrsMark ChrsMark left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ChrsMark

ChrsMark commented Jul 2, 2026

Copy link
Copy Markdown
Member

@dhruv-shah-sumo your input would be appreciated
@TylerHelmuth @dmitryax PTAL so we can proceed with that one since it's open for a while now :)

@ChrsMark

ChrsMark commented Jul 7, 2026

Copy link
Copy Markdown
Member

I'm planning to merge this by EOW if there are no objections.

@ChrsMark

Copy link
Copy Markdown
Member

@kangyili could you please rebase this one? Once it's rebased we can merge it.

@dhruv-shah-sumo

Copy link
Copy Markdown
Contributor

@dhruv-shah-sumo your input would be appreciated

Apologies for the late response. I was away for the last few weeks. The changes look good to me. No concerns in terms of checkpointer method changes.

Resolved conflict in receiver/k8sobjectsreceiver/receiver.go: kept our
error handling for startObserver while incorporating upstream's mu.Lock()
addition before stopperChanList append.

Assisted-by: Claude Sonnet 4.6
@ChrsMark
ChrsMark merged commit 8777bb4 into open-telemetry:main Jul 13, 2026
143 checks passed
@otelbot

otelbot Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution @kangyili! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

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.

6 participants