Fix dialyzer issues and use :ok tuples when :error tuples are possible

[GregMefford]

When running mix dialyzer, I was getting the following errors:

lib/span.ex:87: Function new/1 has no local return
lib/span.ex:88: The call 'Elixir.Spandex.Span':update(#{'__struct__'=>'Elixir.Spandex.Span', 'completion_time'=>'nil', 'env'=>'nil', 'error'=>'nil', 'http'=>'nil', 'id'=>'nil', 'name'=>'nil', 'parent_id'=>'nil', 'private'=>'nil', 'resource'=>'nil', 'service'=>'nil', 'sql_query'=>'nil', 'start'=>'nil', 'tags'=>'nil', 'trace_id'=>'nil', 'type'=>'nil'},_opts@1::any(),#{'__struct__':='Elixir.Optimal
.Schema', 'annotations':=[], 'custom':=[], 'defaults':=[{'private',[]} | {'services',[]} | {'tags',[]},...], 'describe':=[], 'extra_keys?':='true', 'opts':=[atom(),...], 'required':=['env' | 'id' | 'name' | 'service' | 'start' | 'trace_id',...], 'types':=[{atom(),'any' | 'atom' | 'keyword' | 'string' | ['atom' | 'string',...]},...]}) will never return since it differs in the 1st argument from th
e success typing arguments: (#{'__struct__':='Elixir.Spandex.Span', 'completion_time':=_, 'env':=binary(), 'error':='nil' | [{_,_}], 'http':='nil' | [{_,_}], 'id':=_, 'name':=binary(), 'parent_id':=_, 'private':=[{_,_}], 'resource':=binary(), 'service':=atom(), 'sql_query':='nil' | [{_,_}], 'start':=_, 'tags':='nil' | [{_,_}], 'trace_id':=_, 'type':=atom()},any(),#{'__struct__':='Elixir.Optimal.
Schema', 'annotations':=[{_,_}], 'custom':=[{_,_}], 'defaults':=[{_,_}], 'describe':=[{_,_}], 'extra_keys?':=boolean(), 'opts':=[atom()], 'required':=[atom()], 'types':=[{_,_}]})
lib/spandex.ex:264: Function span/4 has no local return

Those are now resolved, but when I started to tug on that thread, the changes ended up leading me all over the place and I'm sorry for the huge PR.

As mentioned in #62, this also standardizes on {:ok, result} tuples wherever an {:error, reason} tuple is possible. For that reason, I'm proposing a version bump from 2.0.0 to 2.1.0 since it's a small breaking change to the API, but my guess is that people probably aren't often using the return values from these functions in practice, as long as the right things are getting passed around inside Spandex.

[GregMefford]

I also put in a commit to set the formatter line_length to 120 because the default of 80 isn't very wide and makes some lines split in annoying ways. I'm fine if you don't want to change that, though.

[asummers]

Should pull in the newest Dialyxir RC while you're here to get the new Elixir formatting =)

[zachdaniel]

@asummers lets do a separate PR with that. Feel free to, or to lodge an issue for me to do it :)

[zachdaniel]

This looks great! Left some feedback/discussion.

[GregMefford]

Hadn't heard about that @asummers, but now I want to look into that! 😁

[zachdaniel]

The testing looks awesome. I'm very surprised to hear the opts with invalid types are ignored. If that's true, it's 100% a bug that we should fix. I still have a slight issue with Span.new I think. When I'm at my laptop I'll post an alternative. Aside from that, and the issue w/ validation, I think this should be good to go.

[zachdaniel]

We should diagnose the validation issue at least, and then maybe fix it in a separate PR.

[zachdaniel]

Ah, I see the issue. The validator is doing the right thing, but we always return the span, even if we didn't update it. I think that, as part of this return type refactor, we would expect to return the errors.

[zachdaniel]

Thinking about it now, there are certain functions where that would be hard (like update_all_spans). I think that, for now, we should leave the tests you've added in place, but have them skipped. I will add a ticket now to investigate replacing the usage of update_or_keep with something that would provide more meaningful returns. We will probably just have to have update_all_spans always return a success, or not update anything in the case that any of them are an error. Either way, probably should not address here.

[zachdaniel]

Alright, @GregMefford since we know whats up with the validation issue the only thing left is Span.new/2. This PR https://github.com/spandex-project/spandex/pull/66 proposes changes to this branch to address my concerns. Once we're done with that, we should be good to go.

[GregMefford]

Ok, I think this is all set. ✅