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

Use development logging when running a local build #889

Merged
merged 3 commits into from
Apr 29, 2020
Merged

Use development logging when running a local build #889

merged 3 commits into from
Apr 29, 2020

Conversation

jrcamp
Copy link
Contributor

@jrcamp jrcamp commented Apr 29, 2020

The zap development logger provides nicer console output as well as a few other different defaults for development.

To decide whether to use production or development it's checking if Version
has been set at build time. If not it assumes it's development. It may be
better not to conflate version number with build type in which case we could
add a separate build type. But first I wanted to see if the general idea
desirable.


Before

{"level":"info","ts":1588175559.4787421,"caller":"service/service.go:360","msg":"Starting OpenTelemetry Contrib Collector...","Version":"latest","GitHash":"<NOT PROPERLY GENERATED>","NumCPU":12}
{"level":"info","ts":1588175559.478791,"caller":"service/service.go:199","msg":"Setting up own telemetry..."}
{"level":"info","ts":1588175559.4789288,"caller":"service/telemetry.go:94","msg":"Serving Prometheus metrics","address":"localhost:8888","legacy_metrics":true,"new_metrics":false,"level":3,"service.instance.id":""}
{"level":"info","ts":1588175559.479254,"caller":"service/service.go:237","msg":"Loading configuration..."}
{"level":"info","ts":1588175559.480563,"caller":"service/service.go:248","msg":"Applying configuration..."}
{"level":"info","ts":1588175559.489289,"caller":"service/service.go:269","msg":"Starting extensions..."}
{"level":"info","ts":1588175559.489312,"caller":"builder/extensions_builder.go:53","msg":"Extension is starting...","component_kind":"extension","component_type":"k8s_observer","component_name":"k8s_observer"}
{"level":"info","ts":1588175559.4897852,"caller":"builder/extensions_builder.go:59","msg":"Extension started.","component_kind":"extension","component_type":"k8s_observer","component_name":"k8s_observer"}
{"level":"info","ts":1588175559.489795,"caller":"builder/extensions_builder.go:53","msg":"Extension is starting...","component_kind":"extension","component_type":"zpages","component_name":"zpages"}
{"level":"info","ts":1588175559.490153,"caller":"zpagesextension/zpagesextension.go:45","msg":"Starting zPages extension","component_kind":"extension","config":{"TypeVal":"zpages","NameVal":"zpages","Disabled":false,"Endpoint":"localhost:55679"}}
{"level":"info","ts":1588175559.490222,"caller":"builder/extensions_builder.go:59","msg":"Extension started.","component_kind":"extension","component_type":"zpages","component_name":"zpages"}
{"level":"warn","ts":1588175559.490238,"caller":"builder/exporters_builder.go:209","msg":"Exportee is not associated with any pipeline and will not export data.","component_kind":"exporter","component_type":"logging","component_name":"logging"}
{"level":"info","ts":1588175559.490324,"caller":"builder/exporters_builder.go:252","msg":"Exporter is enabled.","component_kind":"exporter","exporter":"file"}
{"level":"info","ts":1588175559.490348,"caller":"builder/exporters_builder.go:252","msg":"Exporter is enabled.","component_kind":"exporter","exporter":"signalfx"}

After

2020-04-29T12:01:20.926-0400	INFO	service/service.go:360	Starting OpenTelemetry Contrib Collector...	{"Version": "latest", "GitHash": "<NOT PROPERLY GENERATED>", "NumCPU": 12}
2020-04-29T12:01:20.926-0400	INFO	service/service.go:199	Setting up own telemetry...
2020-04-29T12:01:20.926-0400	INFO	service/telemetry.go:95	Serving Prometheus metrics	{"port": 8888, "legacy_metrics": true, "new_metrics": false, "level": 3, "service.instance.id": ""}
2020-04-29T12:01:20.927-0400	INFO	service/service.go:237	Loading configuration...
2020-04-29T12:01:20.928-0400	INFO	service/service.go:248	Applying configuration...
2020-04-29T12:01:20.937-0400	INFO	service/service.go:269	Starting extensions...
2020-04-29T12:01:20.937-0400	INFO	builder/extensions_builder.go:53	Extension is starting...	{"component_kind": "extension", "component_type": "zpages", "component_name": "zpages"}
2020-04-29T12:01:20.938-0400	INFO	zpagesextension/zpagesextension.go:45	Starting zPages extension	{"component_kind": "extension", "config": {"TypeVal":"zpages","NameVal":"zpages","Disabled":false,"Endpoint":"localhost:55679"}}
2020-04-29T12:01:20.938-0400	INFO	builder/extensions_builder.go:59	Extension started.	{"component_kind": "extension", "component_type": "zpages", "component_name": "zpages"}
2020-04-29T12:01:20.938-0400	INFO	builder/extensions_builder.go:53	Extension is starting...	{"component_kind": "extension", "component_type": "k8s_observer", "component_name": "k8s_observer"}
2020-04-29T12:01:20.938-0400	INFO	builder/extensions_builder.go:59	Extension started.	{"component_kind": "extension", "component_type": "k8s_observer", "component_name": "k8s_observer"}
2020-04-29T12:01:20.938-0400	INFO	builder/exporters_builder.go:252	Exporter is enabled.	{"component_kind": "exporter", "exporter": "signalfx"}
2020-04-29T12:01:20.938-0400	INFO	builder/exporters_builder.go:252	Exporter is enabled.	{"component_kind": "exporter", "exporter": "file"}

The [zap development
logger](https://godoc.org/go.uber.org/zap#NewDevelopmentConfig) provides nicer
console output as well as a few other different defaults for development.

To decide whether to use production or development it's checking if `Version`
has been set at build time. If not it assumes it's development. It may be
better not to conflate version number with build type in which case we could
add a separate build type. But first I wanted to see if the general idea
desirable.
Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

Nice, I really like the idea. As you said it will be better to have it separated from version, so another variable to control for that and it LGTM.

@owais
Copy link
Contributor

owais commented Apr 29, 2020

+1 can we make it a CLI flag so same build can do both?

@jrcamp jrcamp requested a review from pjanotti April 29, 2020 20:01
@jrcamp
Copy link
Contributor Author

jrcamp commented Apr 29, 2020

Err sorry, still have to add the separate control var so ignore the review request temporarily. :)

@codecov-io
Copy link

Codecov Report

Merging #889 into master will decrease coverage by 0.04%.
The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
- Coverage   85.99%   85.94%   -0.05%     
==========================================
  Files         166      166              
  Lines       12980    12993      +13     
==========================================
+ Hits        11162    11167       +5     
- Misses       1396     1403       +7     
- Partials      422      423       +1     
Impacted Files Coverage Δ
internal/version/version.go 10.52% <50.00%> (+10.52%) ⬆️
service/logger.go 68.42% <55.55%> (-11.58%) ⬇️
receiver/opencensusreceiver/octrace/opencensus.go 90.00% <0.00%> (-3.34%) ⬇️
translator/internaldata/resource_to_oc.go 76.47% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71202e4...0d22e21. Read the comment docs.

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

LGTM.

)

const (
logLevelCfg = "log-level"
logLevelCfg = "log-level"
logProfileCfg = "log-profile"
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we want to make this more generic, ie.: use profile for other things but this is already a good start. Let's keep as it is at least for now.

@pjanotti pjanotti merged commit 2cc9fd9 into open-telemetry:master Apr 29, 2020
@jrcamp jrcamp deleted the dev-logging branch April 30, 2020 00:09
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
* Use development logging when running a local build

The [zap development
logger](https://godoc.org/go.uber.org/zap#NewDevelopmentConfig) provides nicer
console output as well as a few other different defaults for development.

To decide whether to use production or development it's checking if `Version`
has been set at build time. If not it assumes it's development. It may be
better not to conflate version number with build type in which case we could
add a separate build type. But first I wanted to see if the general idea
desirable.

* add log profile flag

* add build variable
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
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.

4 participants