raystack / salt Goto Github PK
View Code? Open in Web Editor NEWSalt is a collection of libraries and tools used in the Raystack ecosystem to improve the experience of developing projects with Go.
License: Apache License 2.0
Salt is a collection of libraries and tools used in the Raystack ecosystem to improve the experience of developing projects with Go.
License: Apache License 2.0
Is your feature request related to a problem? Please describe.
grpc-middleware has support of logrus logger. However, they only accept argument with type *logrus.Logger
. In salt, *logrus.Logger
is private, we need to have a getter function to get the object.
Describe the solution you'd like
In this file, we could add a new function
func (l *Logrus) LogrusLogger() *logrus.Logger {
return l.log
}
Is your feature request related to a problem? Please describe.
The current salt logger interface doesn't support grpc loggerV2 interface. Due to which in shield, we have multiple loggers at different layers operating instead of one standard logger across.
Describe the solution you'd like
Make salt logger compatible with grpc logger by refactoring the existing salt logger interface. A few of the methods have to be renamed in salt logger to be able to inject into grpc context so that we could use this logger with custom logging rules.
Add a version package in salt which provides information if a go build is the latest version or not. This package should handle the need of just one binary(CLI) for example meteor as well as CLI and server, as an example Optimus.
Is your feature request related to a problem? Please describe.
The contextual logging with zap in salt is inside log
package. Getting zap log from context would be log.FromContext(ctx)
and return Zap. Since log
has multiple possible logger (logrus & zap), this would be confusing.
Describe the solution you'd like
Add more specific identifier for Zap logger specific function to make it clearer.
log.FromContext
> log.ZapFromContext
log.WithFields
> log.ZapContextWithFields
Is your feature request related to a problem? Please describe.
The printer.Spin()
function assumes writer is STDOUT and does not allow overriding it. Showing spinner in STDOUT can cause issues when the program also writes other useful data (may be structured) to STDOUT.
# This type of usage will be common but with spinner on STDOUT, it may end up writing some
# residual characters used for the spinner rendering into the file also.
$ myprogram-using-spinner dosomework > foo.json
Describe the solution you'd like
Usually, it's better to use STDERR in for logs, progress updates, etc. or at-least if the writer itself is configurable, the client program can override the writer to implement any behaviour (no-op, write-to-stderr, write-to-stdout, etc.)
Note: The printer
package seem to be assuming STDOUT for almost everything. Also, formatting functions encode into byte slice and then print. Instead, all these can be refactored to accept writer and stream-encode (e.g., json.NewEncoder(writer).Encode(v)
)
When running the app without config file, I getting unable to read config file
error instead of ConfigFileNotFoundError
struct.
So i am not able to catch the error in my application side.
if err := loader.Load(&cfg); err != nil {
if errors.As(err, &config.ConfigFileNotFoundError{}) {
fmt.Println(err)
} else {
panic(err)
}
}
The error doesn't go to if condition
Root cause of this issue is https://github.com/odpf/salt/blob/1aad792723ce5e7e0b2d55f676ba0d384b9ea09d/config/config.go#L117
due to !ok
the error is going in else case.
Right now cmdx help
provides hard coded grouping with core commands and additional commands. This can be mad flexible to dynamically group commands based on the group name provided in cobra annotations.
In the config package the default path to load configuration from is set to ./
at https://github.com/odpf/salt/blob/main/config/config.go#L135
Which means that by default if the config is available in the ./
directory it would always load from that first.
This is not be suitable for all use cases as people might want different search order, and can cause confusion when trying to set more config paths.
While currently there is a way to override this its too cumbersome, that is to inject a custom viper instance and set the other defaults on it manually.
Removing this default and letting users use config.WithPathOption
for the same will make it more clear to the users.
raystack/guardian#155
audit logger pkg was initially created in guardian. Need to move it to salt so it is reusable by everyone else.
config read
, write
use yaml format while load
uses salt/config
which internally uses viper
and mapstructure
for processing configs.
This creates an issue that configs written via write
cannot always be properly loaded by load function.
And read
doesn't necessarily display all set configs.
Is your feature request related to a problem? Please describe.
With the current exceptions message, it's hard to get understand which part of the service is failing like data-layer, logic-layer, calling another service etc.
Describe the solution you'd like
Clear and concise details of error so it could help to understand what is the problem.
https://github.com/google/go-github/blob/master/github/github.go#L770
Describe alternatives you've considered
Create a custom Error type to add details about caller, component, request body etc. This custom type will have two different implementations for Error() methods:
Is your feature request related to a problem? Please describe.
A lot of ODPF services are commonly using SQL database with Postgres driver. Most of these services end up repeating the same code to init database and define migrations.
Describe the solution you'd like
Salt can provide a shared sqlx package that provides a client for SQL database and generic methods for running migrations.
Currently salt/log
package tries to abstract Logrus and Zap into a common formatted-logger interface (i.e., Infof(msg string, args ...any)
, etc. ). While logrus
is designed as a formatted-logger, uber/zap
is specifically designed for efficient structured logging and this kind of abstraction nullifies the major benefit of it.
I propose we remove this abstraction altogether1 and assume direct usage of zap within salt and in ODPF applications that use salt. Benefits of doing this:
mux
package that automatically logs request info (method, path, client-ip, etc.) and response info (status, response time, etc.)req.Context()
so that every log in all the subsequent layers automatically add this to every log entry.We can still have some utility functions if we need to (e.g., a helper to inject log context into ctx
). But attempting to abstract over logging functionality will not have justifiable benefits. โฉ
Scope/Type: config
Related discussion here.
I think that salt should have the ability to export environment variables and their respective values through a helper function; the closest we can get as of now is through the getFlattenedStructKeys function, which allows us to get keys. (which is private in nature, as it is an utility function)
It could be helpful in situations like this, where we need to get env vars. (salt could help in exporting values (more specifically, env variable key-value pair) into a map[string]string
, which can then be used in populateData
in meteor.)
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.