Skip to content

Comments

refactor: switch cachex stats to telemetry polling#3156

Open
ruslandoga wants to merge 5 commits intoLogflare:mainfrom
ruslandoga:rd/cachex-stats
Open

refactor: switch cachex stats to telemetry polling#3156
ruslandoga wants to merge 5 commits intoLogflare:mainfrom
ruslandoga:rd/cachex-stats

Conversation

@ruslandoga
Copy link
Contributor

ANL-1033

Removed log-based Cachex stats collection in favor of telemetry-based polling system.

["Logflare", context, "Cache"] = Module.split(cache)
{cache, context |> Macro.underscore() |> String.to_atom()}
end)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This evaluates to

[
  {Logflare.TeamUsers.Cache, :team_users},
  {Logflare.Partners.Cache, :partners},
  {Logflare.Users.Cache, :users},
  {Logflare.Backends.Cache, :backends},
  {Logflare.Sources.Cache, :sources},
  {Logflare.Billing.Cache, :billing},
  {Logflare.SourceSchemas.Cache, :source_schemas},
  {Logflare.Auth.Cache, :auth},
  {Logflare.Endpoints.Cache, :endpoints},
  {Logflare.Rules.Cache, :rules},
  {Logflare.SavedSearches.Cache, :saved_searches}
]

or should it be hardcoded?

Copy link
Contributor

Choose a reason for hiding this comment

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

can you inverse the logic so that we can do list_contexts_to_cache() and in list_caches it will call list_contexts_to_cache/0 and concat the .Cache

Copy link
Contributor

Choose a reason for hiding this comment

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

the module.split isn't very pretty 😆

Copy link
Contributor

Choose a reason for hiding this comment

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

besides that, its fine with keeping it dynamic.

@ruslandoga
Copy link
Contributor Author

@Ziinc 👋

I think this is ready for review.

Copy link
Contributor

@Ziinc Ziinc left a comment

Choose a reason for hiding this comment

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

just minor refactoring for cleaner code

["Logflare", context, "Cache"] = Module.split(cache)
{cache, context |> Macro.underscore() |> String.to_atom()}
end)

Copy link
Contributor

Choose a reason for hiding this comment

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

can you inverse the logic so that we can do list_contexts_to_cache() and in list_caches it will call list_contexts_to_cache/0 and concat the .Cache

["Logflare", context, "Cache"] = Module.split(cache)
{cache, context |> Macro.underscore() |> String.to_atom()}
end)

Copy link
Contributor

Choose a reason for hiding this comment

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

the module.split isn't very pretty 😆

["Logflare", context, "Cache"] = Module.split(cache)
{cache, context |> Macro.underscore() |> String.to_atom()}
end)

Copy link
Contributor

Choose a reason for hiding this comment

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

besides that, its fine with keeping it dynamic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants