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.

Sign up for GitHub

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

Jump to bottom

Add Environment and Services support #196

Merged
merged 56 commits into from
Mar 23, 2022
Merged

Add Environment and Services support #196

merged 56 commits into from
Mar 23, 2022

Conversation

MikeGoldsmith
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith commented Mar 18, 2022 •
edited by JamieDanielson

Which problem is this PR solving?

Short description of the changes

If using an environment-aware key:

  • dataset = serviceName
  • add field of service.name in addition to service_name to be more consistent with otel
  • use default service name of unknown_service:<processname> or unknown_service:ruby if process name is unavailable and no service name provided
  • don't propagate dataset by default unless legacy key (to prevent overwriting of service name)
  • warn on missing service name and api key
  • warn on presence of dataset, noting data will be sent to service name
  • if dataset derived from service name starts with unknown_service*, truncate to unknown_service for dataset
  • if dataset derived from service name has extra whitespace, trim whitespace and console warn on diff

warning provided if dataset is set in config when using environment key:
dataset will be ignored, sending data to message-rb
service_name is unknown, will set to unknown_service:rackup
dataset will be ignored, sending data to unknown_service:rackup

If using a classic key:

  • dataset is dataset in config, no default set
  • add field of service.name in addition to service_name to be more consistent with otel
  • use default service name of unknown_service:<processname> or unknown_service:go if process name is unavailable and no service name provided
  • dataset propagates by default
  • warn on missing service name, api key, dataset

warning on missing env vars with classic key:
empty service_name option
empty dataset option
Libhoney::Client: no dataset configured, disabling sending events

@MikeGoldsmith MikeGoldsmith changed the title Mike/env service Add Environment and Services support Mar 18, 2022
MikeGoldsmith and others added 13 commits March 18, 2022 14:41
@MikeGoldsmith
make classic a predicate method
286fd2b
@MikeGoldsmith
update test names
a5ce1a4
@MikeGoldsmith
appease rubocop by shortening test name
0548d1b
@MikeGoldsmith
make rubocop line length 100
2d4f950
@MikeGoldsmith
remove explict linelength overrides
c37a3a7
@MikeGoldsmith
appease more linting
f458e3d
@MikeGoldsmith
add another test case
2dea355
@MikeGoldsmith
rework dataset accessor
2e0a6e5
@robbkidd @JamieDanielson
reorganize Configuration
d9cdba0 
+ tighten up checks and give them names
+ reorganize the tests to assume new Honeycomb world and treat classic
  like the special case

Co-authored-by: JamieDanielson <JamieDanielson@users.noreply.github.com>
@robbkidd @JamieDanielson
new propagators for environments world
de15302 
Don't pass dataset around. Not complete. Still need to sort out the
Honeycomb::Propagation::Parser & ::Serializer which are the modules
mixed in to Trace.

Co-authored-by: Jamie Danielson <jamiedanielson@honeycomb.io>
@JamieDanielson @robbkidd
wip
e3ba977 
Co-authored-by: Robb Kidd <robb@thekidds.org>
@JamieDanielson
remove unnecessary import
16e5669
@JamieDanielson
assert service_name and service.name fields exist
49e67bb
@JamieDanielson JamieDanielson marked this pull request as ready for review March 22, 2022 22:41
@JamieDanielson JamieDanielson requested a review from a team March 22, 2022 22:41
@JamieDanielson JamieDanielson self-assigned this Mar 22, 2022
@JamieDanielson JamieDanielson added type: enhancement New feature or request version: bump minor A PR that adds behavior, but is backwards-compatible. labels Mar 22, 2022
vreynolds
vreynolds approved these changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@vreynolds vreynolds left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple small notes

spec/generators/honeycomb/honeycomb_generator_spec.rb Outdated Show resolved Hide resolved
spec/honeycomb/configuration_spec.rb Outdated Show resolved Hide resolved
@JamieDanielson @vreynolds
remove commented line
3ce22f9 
Co-authored-by: Vera Reynolds <vreynolds@users.noreply.github.com>
robbkidd
robbkidd approved these changes Mar 23, 2022
View reviewed changes
Copy link
Member

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

:shipit:

@JamieDanielson JamieDanielson merged commit 590c422 into main Mar 23, 2022
@JamieDanielson JamieDanielson deleted the mike/env-service branch March 23, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamieDanielson JamieDanielson JamieDanielson left review comments

@robbkidd robbkidd robbkidd approved these changes

@vreynolds vreynolds vreynolds approved these changes

@martin308 martin308 Awaiting requested review from martin308

Assignees

@JamieDanielson JamieDanielson

Labels
type: enhancement New feature or request version: bump minor A PR that adds behavior, but is backwards-compatible.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update for E&S
4 participants
@MikeGoldsmith @robbkidd @vreynolds @JamieDanielson