Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Project telemetry data #418

Merged

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Nov 2, 2021

This PR adds a Projector for the required Telemetry information.

HostName
SLESVersion
CPUCount
SocketCount
TotalMemoryMB
CloudProvider

This works also as a PoC for projecting the Hosts as a replacement for Consul Catalog.


Extra:

By adding a

_ "github.com/trento-project/trento/test"

we can consider the cwd for our tests being the project root, which allows to simplify path specification in tests and allows us to reuse mocks.

Example

NewDiscoveredCloudMock in agent/discovery/mocks/discovered_cloud_mock.go is used as a source for mocks in two different places

  • agent/discovery/collector/publishing_test.go
  • web/datapipeline/telemetry_projector_test.go

Without the trick it would't have been possible to reuse the same NewDiscoveredCloudMock because, in respect of the caller executor (publishing_test.go or telemetry_projector_test.go) it would have required to load the same file from a different relative location.

So,
calling NewDiscoveredCloudMock from agent/discovery/collector/publishing_test.go would require loading the fixture from ../../../test/fixtures/discovery/azure/azure_discovery.json

while calling NewDiscoveredCloudMock from web/datapipeline/telemetry_projector_test.go would require loading the fixture from ../../test/fixtures/discovery/azure/azure_discovery.json

Note the difference ../../../ vs ../../

with this trick we can consider the cwd as the root of the project so the path to be loaded would always be the same
./test/fixtures/discovery/azure/azure_discovery.json

Hope it makes sense.


What's next:
For telemetry: spike with the team on opentelemetry/promscale/timescale
For consul migration: this opens doors for consul Host catalog replacement by projecting hosts and defining Heartbeat and healthcheck

Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Hey @nelsonkopliku ,
I have put a bunch of comments, but I'm just setting the PR to request changes for the subscription data usage, which I think we should change. Besides this one, everything else is just a opinionated comment hehe

agent/discovery/collector/client_test.go Show resolved Hide resolved
test/testing.go Show resolved Hide resolved
web/datapipeline/telemetry_projector.go Outdated Show resolved Hide resolved
web/models/telemetry.go Outdated Show resolved Hide resolved
web/datapipeline/projector_registry.go Outdated Show resolved Hide resolved
web/datapipeline/telemetry_projector.go Outdated Show resolved Hide resolved
web/datapipeline/telemetry_projector.go Outdated Show resolved Hide resolved
web/datapipeline/telemetry_projector.go Outdated Show resolved Hide resolved
web/datapipeline/telemetry_projector.go Outdated Show resolved Hide resolved
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Thanks @nelsonkopliku ,
Now we will need to see how we can really use this table hehe

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

🎊🎉🍾

@nelsonkopliku nelsonkopliku merged commit b572b74 into trento-project:main Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of existing features
Development

Successfully merging this pull request may close these issues.

4 participants